Skip to content

feat: add Redis caching plugin to transports with UI configuration#237

Merged
akshaydeo merged 1 commit intomainfrom
08-08-feat_redis_plugin_added_to_transport
Aug 11, 2025
Merged

feat: add Redis caching plugin to transports with UI configuration#237
akshaydeo merged 1 commit intomainfrom
08-08-feat_redis_plugin_added_to_transport

Conversation

@Pratham-Mishra04
Copy link
Copy Markdown
Collaborator

Redis Caching Plugin Integration

This PR adds Redis caching functionality to Bifrost, allowing responses to be cached and retrieved based on request parameters. The implementation includes:

  • Simplified Redis plugin configuration by removing the cache_only_successful option
  • Improved cache key generation by including the cache key in the hash format
  • Added context tracking for cache hits to prevent re-caching already cached responses
  • Created new Redis configuration API endpoints and handlers
  • Added UI components for Redis configuration management
  • Integrated Redis caching with the HTTP transport layer
  • Fixed cache key handling in context values

The caching system works by sending an x-bf-cache-key header with requests to enable caching for specific requests. Cache keys are generated based on request parameters and can be configured to include model and provider information.

The UI now includes a Redis configuration section that appears when caching is enabled, allowing users to configure connection settings, TTL, key prefixes, and caching behavior.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 8, 2025

Caution

Review failed

The pull request is closed.

Summary by CodeRabbit

  • New Features
    • Optional Redis-based request caching with a toggle in Core settings.
    • Per-request cache control via x-bf-cache-key and x-bf-cache-ttl headers.
    • Cache management endpoints to view/update cache settings and delete cache by key.
  • UI
    • New “Enable Caching” section with Redis cache configuration form and restart warning.
    • Improved Redis address validation and inline success/error toasts.
  • Documentation
    • Added Redis cache usage, headers, and examples; updated plugin TTL override docs.
  • Chores
    • Minor workflow notification formatting updates.

Walkthrough

Adds Redis request caching end-to-end: DB and in-memory config, context header parsing, Redis plugin initialization and routes, cache config persistence and UI, validation utilities, and documentation updates (including cache key/TTL headers and cache management endpoints).

Changes

Cohort / File(s) Summary
Config flag & persistence
transports/bifrost-http/lib/config.go, transports/bifrost-http/lib/models.go, transports/bifrost-http/lib/store.go, transports/bifrost-http/handlers/config.go
Adds EnableCaching to ClientConfig/DBClientConfig and propagates it through load/save paths; introduces DBCacheConfig DB model and migrations; adds store methods GetCacheConfig, GetCacheConfigRedacted, UpdateCacheConfig; ConfigHandler now applies EnableCaching from update requests.
Context / request headers
transports/bifrost-http/lib/ctx.go
Parses x-bf-cache-key and x-bf-cache-ttl, storing values in request context (uses redis.ContextKey); TTL parsed as duration or seconds and ignored on parse failure.
HTTP handlers & routing
transports/bifrost-http/handlers/cache.go, transports/bifrost-http/handlers/.../config.go
New CacheHandler (GET/PUT /api/config/cache, DELETE /api/cache/{key}); Update preserves redacted password, validates address and TTL; register cache routes when enabled.
Server bootstrap & plugin wiring
transports/bifrost-http/main.go, transports/go.mod
Conditionally initializes Redis plugin when EnableCaching is true, maps DB cache config to plugin config, appends plugin to loadedPlugins, and wires cache routes; adds redis plugin dependency and replace directives.
UI: config page & component
ui/app/config/page.tsx, ui/components/config/cache-config-form.tsx
Adds enable_caching toggle to core config and conditional CacheConfigForm; new form fetches/updates cache config, uses debounced updates, handles redacted password, toasts, and restart warning.
UI: API & types
ui/lib/api.ts, ui/lib/types/config.ts
Adds CacheConfig type and getCacheConfig / updateCacheConfig methods; CoreConfig gains enable_caching.
UI: validation utilities
ui/lib/utils/validation.ts
Adds isValidRedisAddress validating host:port, [ipv6]:port, redis:// and rediss:// forms with host/port checks.
Docs & plugin README
docs/usage/http-transport/endpoints.md, plugins/redis/README.md
Documents Redis Cache Management endpoints, header usage, TTL formats, and CacheTTLKey option in Redis plugin README. (Some duplicated sections present.)
Workflows
.github/workflows/*
Minor formatting changes to Discord notification messages (triple backticks).

Sequence Diagram(s)

sequenceDiagram
  participant Admin
  participant UI
  participant API as HTTP Server
  participant Store as ConfigStore
  participant Redis as Redis Plugin

  Admin->>UI: Toggle enable_caching / edit cache config
  UI->>API: PUT /api/config/cache (CacheConfig)
  API->>Store: UpdateCacheConfig(config)
  Store-->>API: OK
  API-->>UI: { config }
  Note over API,Redis: On server start, if EnableCaching=true → init Redis plugin and register cache routes
Loading
sequenceDiagram
  participant Client
  participant API as HTTP Server
  participant Ctx as Context Builder
  participant Redis as Redis Plugin
  participant Upstream as Backend

  Client->>API: Request with x-bf-cache-key / x-bf-cache-ttl
  API->>Ctx: ConvertToBifrostContext(headers)
  Ctx-->>API: Context with cache key & ttl
  API->>Redis: Get(key)
  alt Cache hit
    Redis-->>API: Cached response
    API-->>Client: Cached response (bifrost_cached flag)
  else Cache miss
    API->>Upstream: Execute request
    Upstream-->>API: Response/stream
    API->>Redis: Set(key, response, ttl)
    API-->>Client: Response
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • adds zerolog logger #251 — Shared changes to transports/bifrost-http main wiring and logger usage that overlap with this PR's main.go modifications.

Poem

I flipped the switch and dug a den,
Carrots of keys stored now and then. 🥕
TTLs tick softly, cached and neat,
Redis burrows, fast and sweet.
Hop—requests return in a blink of beat.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c938aa0 and 78ee9ed.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (17)
  • .github/workflows/core-dependency-update.yml (1 hunks)
  • .github/workflows/main-branch-notifications.yml (1 hunks)
  • docs/usage/http-transport/endpoints.md (1 hunks)
  • plugins/redis/README.md (1 hunks)
  • transports/bifrost-http/handlers/cache.go (1 hunks)
  • transports/bifrost-http/handlers/config.go (1 hunks)
  • transports/bifrost-http/lib/config.go (1 hunks)
  • transports/bifrost-http/lib/ctx.go (2 hunks)
  • transports/bifrost-http/lib/models.go (2 hunks)
  • transports/bifrost-http/lib/store.go (6 hunks)
  • transports/bifrost-http/main.go (3 hunks)
  • transports/go.mod (3 hunks)
  • ui/app/config/page.tsx (3 hunks)
  • ui/components/config/cache-config-form.tsx (1 hunks)
  • ui/lib/api.ts (2 hunks)
  • ui/lib/types/config.ts (1 hunks)
  • ui/lib/utils/validation.ts (1 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 08-08-feat_redis_plugin_added_to_transport

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Collaborator Author

Pratham-Mishra04 commented Aug 8, 2025

@Pratham-Mishra04 Pratham-Mishra04 marked this pull request as ready for review August 8, 2025 12:56
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 08-08-feat_redis_plugin_added_to_transport branch from 48692d6 to 8f7ce6c Compare August 8, 2025 13:01
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

🔭 Outside diff range comments (2)
transports/bifrost-http/handlers/config.go (1)

56-59: Update function comment to reflect broader updatable fields

Comment still states only drop_excess_requests is hot-reloaded. It now also persists EnableCaching and other flags here. Please update to avoid confusion.

// handleUpdateConfig updates core configuration settings.
// Note: Some fields take effect immediately (e.g., DropExcessRequests); others may require restart depending on initialization paths (e.g., plugin-related flags like EnableCaching).
plugins/redis/main.go (1)

138-138: Log level inconsistency for connection success.

The successful Redis connection message is logged at Info level, but all other informational messages use Debug level. Consider using Debug for consistency.

-logger.Info(fmt.Sprintf("%s Successfully connected to Redis at %s", PluginLoggerPrefix, config.Addr))
+logger.Debug(fmt.Sprintf("%s Successfully connected to Redis at %s", PluginLoggerPrefix, config.Addr))
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 48692d6 and 8f7ce6c.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (14)
  • plugins/redis/main.go (10 hunks)
  • transports/bifrost-http/handlers/config.go (1 hunks)
  • transports/bifrost-http/handlers/redis.go (1 hunks)
  • transports/bifrost-http/lib/config.go (1 hunks)
  • transports/bifrost-http/lib/ctx.go (2 hunks)
  • transports/bifrost-http/lib/models.go (2 hunks)
  • transports/bifrost-http/lib/store.go (6 hunks)
  • transports/bifrost-http/main.go (3 hunks)
  • transports/go.mod (3 hunks)
  • ui/app/config/page.tsx (3 hunks)
  • ui/components/config/redis-config-form.tsx (1 hunks)
  • ui/lib/api.ts (2 hunks)
  • ui/lib/types/config.ts (1 hunks)
  • ui/lib/utils/validation.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (38)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:183-191
Timestamp: 2025-08-03T20:36:21.906Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers the current string matching approach for determining rate limit violation decision types (DecisionTokenLimited vs DecisionRequestLimited) adequate and not important enough to refactor with more robust explicit checks, preferring functional simplicity over theoretical robustness improvements.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#158
File: .github/workflows/transports-release.yml:30-43
Timestamp: 2025-07-18T11:12:28.861Z
Learning: Pratham-Mishra04 relies on branch protection rules and mandatory code reviews for security in the bifrost repository, preferring process controls over technical security measures like environment variable isolation for GitHub Actions workflows. All commits are reviewed before merging to main branch.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T04:58:08.229Z
Learning: Pratham-Mishra04 prefers not to add logging or error handling for unreachable code paths in the Bifrost project. When provider types or similar entities are predefined in the system, defensive programming like logging in default cases is considered unnecessary overhead.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: docs/usage/go-package/mcp.md:408-412
Timestamp: 2025-07-01T12:40:08.576Z
Learning: Pratham-Mishra04 is okay with keeping bullet list formatting that uses colons after dashes in markdown documentation, even if it triggers linter warnings, preferring functionality over strict formatting rules.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: tests/core-providers/scenarios/chat_completion_stream.go:103-105
Timestamp: 2025-07-16T04:26:09.288Z
Learning: Pratham-Mishra04 prefers to keep test code simple when it serves its basic functional purpose. For tests that are meant to validate core functionality (like verifying streaming works), they consider hard-coded reasonable limits acceptable rather than making them configurable.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:880-910
Timestamp: 2025-07-08T17:14:21.544Z
Learning: Pratham-Mishra04 prefers resilient system design where missing environment variables for MCP connections should not cause complete system failure. The system should continue processing other MCP connections even when some fail, maintaining partial functionality rather than implementing fail-fast behavior.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: transports/README.md:26-28
Timestamp: 2025-07-01T12:45:06.906Z
Learning: Pratham-Mishra04 prefers keeping documentation examples simple and concise, trusting users to handle production-specific considerations like version pinning themselves rather than cluttering examples with additional notes.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:1081-1098
Timestamp: 2025-07-08T17:16:50.811Z
Learning: Pratham-Mishra04 prefers practical redaction approaches over theoretical security improvements when the threat model is low-risk, such as admin-only interfaces in the Bifrost project. Fixed-length redaction is acceptable when only trusted administrators will see the redacted values.
📚 Learning: 2025-08-07T13:33:18.094Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:0-0
Timestamp: 2025-08-07T13:33:18.094Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), the context key for enabling per-request JSON parsing is "enable-streaming-json-parser" (defined as EnableStreamingJSONParser constant), which is distinct from the plugin name constant to avoid confusion between plugin identification and per-request activation control.

Applied to files:

  • transports/bifrost-http/lib/config.go
  • plugins/redis/main.go
📚 Learning: 2025-06-20T16:21:18.912Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-06-27T17:07:39.462Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#135
File: docs/core-package.md:105-116
Timestamp: 2025-06-27T17:07:39.462Z
Learning: In Go, when importing a package, the identifier used to access it is determined by the `package` declaration in the Go source files, not the directory name. For the Bifrost project, the core directory files declare `package bifrost`, so importing `"github.com/maximhq/bifrost/core"` automatically makes it accessible as `bifrost.Init`, `bifrost.Ptr`, etc., without needing an explicit alias.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-07-08T18:09:32.147Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-07-08T18:09:32.147Z
Learning: In the Bifrost HTTP transport logging plugin (transports/bifrost-http/plugins/logging/main.go), the DatabasePath is explicitly set from main.go via a config struct pointing to appDir/logs, so the default value in the NewLoggerPlugin constructor is only used for standalone plugin usage and doesn't affect the main application flow.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-16T03:55:16.949Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T03:55:16.949Z
Learning: Go 1.24 was released in February 2025 and is stable and available for use in go.mod files.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-07-10T13:44:14.518Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.518Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-16T04:29:53.409Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use `package main` for test files in the tests/core-providers directory rather than more descriptive package names like `coreproviders_test`.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-06-16T04:13:55.437Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-08-08T06:51:27.123Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/schemas/bifrost.go:686-692
Timestamp: 2025-08-08T06:51:27.123Z
Learning: In core/schemas/bifrost.go, BifrostError.StreamControl is intended for plugin developers only and is excluded from JSON serialization; its presence controls streaming behavior internally.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-16T04:12:05.427Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-17T14:04:21.104Z
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • plugins/redis/main.go
📚 Learning: 2025-08-04T22:19:12.975Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#212
File: transports/bifrost-http/integrations/openai/types.go:250-288
Timestamp: 2025-08-04T22:19:12.975Z
Learning: In transports/bifrost-http/integrations/openai/types.go, the ConvertToBifrostRequest function for OpenAIEmbeddingRequest handles []interface{} input without validation for non-string elements because Pratham-Mishra04 confirmed this case will never happen in practice, consistent with their preference to avoid defensive programming for unreachable scenarios.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and always returns a pointer to a valid context. It starts with context.Background() and only adds values to it, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-08-03T20:11:26.945Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:220-220
Timestamp: 2025-08-03T20:11:26.945Z
Learning: In the Bifrost HTTP transport handlers using fasthttp router, path parameters extracted via ctx.UserValue() can be safely type-asserted to string without additional checks because the router guarantees their presence and type when routes match. Pratham-Mishra04 confirmed that defensive type assertion checks are unnecessary overhead in this context.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-10T13:51:52.859Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:140-146
Timestamp: 2025-06-10T13:51:52.859Z
Learning: In Bifrost core v1.0.9, ImageContent.Type was a pointer type (*string accessed via bifrost.Ptr), but in v1.0.10 it was changed to a value type (ImageContentType). When reviewing code, check the core version being used to determine the correct assignment pattern.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-10T11:19:29.604Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-15T14:24:49.882Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-10T11:19:29.604Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-07-10T13:44:39.237Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:16-18
Timestamp: 2025-07-10T13:44:39.237Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers to ignore JSON marshaling errors when storing log entries because logging is not critical for their use case and they are certain the marshaling operations won't fail.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-07-10T13:44:23.297Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:94-111
Timestamp: 2025-07-10T13:44:23.297Z
Learning: Pratham-Mishra04 prefers not to add error handling for JSON marshaling operations in the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go) because logging is not critical functionality and the structured schema data being marshaled is unlikely to fail. They accept the risk of not handling json.Marshal errors in logging contexts to keep the code simple.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-08-03T20:53:08.832Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: ui/app/governance/page.tsx:29-60
Timestamp: 2025-08-03T20:53:08.832Z
Learning: In the Bifrost governance page (ui/app/governance/page.tsx), Pratham-Mishra04 intentionally keeps the loading state active when critical errors occur (such as governance being disabled or core config failures) to prevent showing a broken or incomplete governance interface to users. This is a deliberate UX design choice to avoid presenting non-functional page states.

Applied to files:

  • ui/app/config/page.tsx
  • ui/components/config/redis-config-form.tsx
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called at final chunk of streaming responses and when errors are returned, not repeatedly during streaming. This makes the performance impact of the database query minimal and doesn't warrant optimization with in-memory caching.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • plugins/redis/main.go
📚 Learning: 2025-08-03T20:50:48.247Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/tracker.go:146-146
Timestamp: 2025-08-03T20:50:48.247Z
Learning: In the Bifrost governance tracker (transports/bifrost-http/plugins/governance/tracker.go), Pratham-Mishra04 implements database persistence for reset operations using arrays to collect reset entities (resetRateLimits and resetBudgets) and then batch persist them using t.db.Save() with proper error aggregation in the PerformStartupResets method.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-07-08T18:20:24.086Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called in two specific scenarios: when ErrorDetails is present (error case) and when isFinalChunk is true (end of streaming). It is not called repeatedly during streaming, making performance optimization with caching unnecessary.

Applied to files:

  • transports/bifrost-http/main.go
  • plugins/redis/main.go
📚 Learning: 2025-07-08T17:31:44.662Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#145
File: transports/bifrost-http/main.go:124-128
Timestamp: 2025-07-08T17:31:44.662Z
Learning: Pratham-Mishra04 prefers to keep the CORS middleware simple in the Bifrost HTTP transport (transports/bifrost-http/main.go) rather than adding port validation for localhost origins, considering the current implementation sufficient for the intended use case.

Applied to files:

  • transports/bifrost-http/main.go
📚 Learning: 2025-08-03T20:28:00.857Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:1038-1049
Timestamp: 2025-08-03T20:28:00.857Z
Learning: In the Bifrost governance handler (transports/bifrost-http/handlers/governance.go), Pratham-Mishra04 is comfortable with exposing full error details including err.Error() in production responses for governance API endpoints, prioritizing simplicity over potential information disclosure concerns.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-08T07:07:08.801Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/providers/openai.go:459-459
Timestamp: 2025-08-08T07:07:08.801Z
Learning: In core/providers/utils.go, processAndSendResponse and processAndSendError now take exactly five parameters: (ctx, postHookRunner, response|err, responseChan, logger). All providers (OpenAI, Anthropic, Cohere, Bedrock) must call these with the logger as the fifth argument; a repo-wide check in PR maximhq/bifrost#219 confirmed this is already consistent.

Applied to files:

  • ui/lib/api.ts
  • plugins/redis/main.go
📚 Learning: 2025-07-29T16:10:52.088Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#196
File: core/providers/openai.go:180-183
Timestamp: 2025-07-29T16:10:52.088Z
Learning: In the Bifrost provider architecture, `handleProviderResponse` is a utility function that only parses and returns raw response data when the `sendBackRawResponse` flag is true. It's the responsibility of each individual provider (OpenAI, Anthropic, etc.) to conditionally set `response.ExtraFields.RawResponse` using the returned raw response data based on their `sendBackRawResponse` flag. This represents a separation of concerns where the utility handles parsing and the provider handles response object construction.

Applied to files:

  • ui/lib/api.ts
  • plugins/redis/main.go
📚 Learning: 2025-06-09T14:03:34.227Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:0-0
Timestamp: 2025-06-09T14:03:34.227Z
Learning: In the Bifrost HTTP transport layer (transports/bifrost-http/integrations/), request validation like checking for empty messages should be handled by the provider rather than at the transport layer. The transport layer should forward requests to Bifrost core/providers for validation.

Applied to files:

  • ui/lib/api.ts
📚 Learning: 2025-08-02T13:02:13.642Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#174
File: transports/bifrost-http/lib/models.go:251-254
Timestamp: 2025-08-02T13:02:13.642Z
Learning: In the Bifrost project's database models (transports/bifrost-http/lib/models.go), Pratham-Mishra04 considers meta config types to be predefined and expects that unknown meta config types will never occur in practice. The current implementation that returns nil for unknown types in the AfterFind hook is intentional, as defensive programming for this scenario is considered unnecessary.

Applied to files:

  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-07-09T06:55:22.017Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T06:55:22.017Z
Learning: In the Bifrost project's ConfigStore, different functions handle sensitive values differently based on their use cases. The `restoreMetaConfigEnvVars` function preserves actual values without redaction when writing config back to file for round-trip fidelity, while `GetProviderConfigRedacted` redacts sensitive values for external API responses for security purposes. This different handling is intentional and based on the specific context where each function is used.

Applied to files:

  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-08-07T13:33:07.837Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:0-0
Timestamp: 2025-08-07T13:33:07.837Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), TejasGhatte prefers to return responses as-is (skip processing) when a request ID cannot be determined, rather than generating fallback IDs. The getRequestID method should return an empty string when no proper identifier is found, allowing the PostHook method's early return logic to skip accumulation and processing.

Applied to files:

  • plugins/redis/main.go
📚 Learning: 2025-06-09T16:46:32.018Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/router.go:19-33
Timestamp: 2025-06-09T16:46:32.018Z
Learning: In the GenericRouter (transports/bifrost-http/integrations), ResponseFunc is not called if the BifrostResponse parameter is nil, providing built-in protection against nil response marshaling.

Applied to files:

  • plugins/redis/main.go
📚 Learning: 2025-08-04T19:39:39.417Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#209
File: plugins/jsonparser/plugin_test.go:38-45
Timestamp: 2025-08-04T19:39:39.417Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), the PostHook method is designed with a contract that guarantees the response structure (Choices, BifrostStreamResponseChoice, Delta, Content) will always be present when called, making defensive nil checks unnecessary in tests.

Applied to files:

  • plugins/redis/main.go
📚 Learning: 2025-06-14T04:06:58.240Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#79
File: core/bifrost.go:94-103
Timestamp: 2025-06-14T04:06:58.240Z
Learning: In core/bifrost.go, the count parameter in RunPostHooks method is intentionally kept separate from p.executedPreHooks to support circuit breaker plugins that may need to trigger PostHooks for only a subset of executed plugins when detecting failure conditions mid-execution.

Applied to files:

  • plugins/redis/main.go
🧬 Code Graph Analysis (5)
transports/bifrost-http/lib/ctx.go (1)
plugins/redis/main.go (1)
  • ContextKey (197-197)
ui/lib/api.ts (1)
ui/lib/types/config.ts (1)
  • RedisConfig (134-146)
ui/components/config/redis-config-form.tsx (6)
ui/lib/types/config.ts (1)
  • RedisConfig (134-146)
ui/lib/api.ts (1)
  • apiService (506-506)
ui/components/ui/card.tsx (2)
  • Card (50-50)
  • CardContent (50-50)
ui/components/ui/label.tsx (1)
  • Label (24-24)
ui/components/ui/input.tsx (1)
  • Input (7-22)
ui/components/ui/switch.tsx (1)
  • Switch (37-37)
transports/bifrost-http/lib/store.go (2)
transports/bifrost-http/lib/models.go (2)
  • DBRedisConfig (134-146)
  • DBRedisConfig (155-155)
transports/bifrost-http/lib/config.go (1)
  • ClientConfig (11-20)
plugins/redis/main.go (1)
transports/bifrost-http/lib/ctx.go (1)
  • ContextKey (59-59)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
🔇 Additional comments (5)
transports/bifrost-http/lib/config.go (1)

18-18: EnableCaching field addition looks correct and consistent

JSON tag style matches existing fields; comment is clear. No further issues here.

transports/go.mod (1)

12-12: Redis plugin path and cache key validated

  • Verified plugins/redis/go.mod declares
    module github.com/maximhq/bifrost/plugins/redis, matching the require path.
  • Confirmed "request-cache-key" is used consistently in both
    transports/bifrost-http/main.go and transports/bifrost-http/lib/ctx.go.

Note: In a monorepo this setup is fine; if you plan to publish the transport module independently, drop any relative replace directives to preserve reproducible builds.

transports/bifrost-http/lib/ctx.go (2)

15-15: Importing redis.ContextKey keeps context key types consistent

This mirrors the existing maxim import pattern and ensures the plugin can retrieve the value. Good choice.


115-118: Document the x-bf-cache-key header in the special-headers list and verify plugin usage

The code correctly stores x-bf-cache-key in context under redis.ContextKey("request-cache-key").
Please:

  • Add x-bf-cache-key to the comment block above that enumerates special headers for completeness.
  • Double-check that any downstream plugins consume this key using the same redis.ContextKey("request-cache-key") (no matches found under plugins/).
ui/lib/types/config.ts (1)

129-146: Types look accurate

enable_caching field and RedisConfig interface mirror the backend schema cleanly.

Comment thread plugins/redis/main.go
Comment thread transports/bifrost-http/handlers/config.go
Comment thread transports/bifrost-http/handlers/redis.go Outdated
Comment thread transports/bifrost-http/lib/models.go Outdated
Comment thread transports/bifrost-http/lib/store.go Outdated
Comment thread ui/app/config/page.tsx
Comment thread ui/components/config/redis-config-form.tsx Outdated
Comment thread ui/components/config/redis-config-form.tsx Outdated
Comment thread ui/lib/api.ts Outdated
Comment thread ui/lib/utils/validation.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 19

🔭 Outside diff range comments (4)
transports/bifrost-http/main.go (1)

131-133: CORS: Allow the new X-BF-Cache-Key header (and optionally X-BF-VK)

Clients sending x-bf-cache-key from browsers will fail preflight unless it’s whitelisted.

Apply:

- ctx.Response.Header.Set("Access-Control-Allow-Headers", "Content-Type, Authorization, X-Requested-With")
+ ctx.Response.Header.Set("Access-Control-Allow-Headers", "Content-Type, Authorization, X-Requested-With, X-BF-Cache-Key, X-BF-VK")
transports/bifrost-http/lib/store.go (2)

700-703: saveClientConfigToDB drops + recreates row – potential race.

DELETE …; CREATE is non-atomic. Two concurrent updates can interleave and end up with duplicate rows.
Consider wrapping in a transaction with REPLACE INTO / ON CONFLICT DO UPDATE to guarantee a single row.


2085-2112: UpdateRedisConfig is not transactional and can lose writes.

Two requests racing:

  1. both DELETE all rows
  2. both CREATE – final table holds 2 rows

Wrap the delete+create in db.Transaction or issue a single Save() with a unique PK to enforce one-row invariant.

return s.db.Transaction(func(tx *gorm.DB) error {
    if err := tx.Delete(&DBRedisConfig{}, "1 = 1").Error; err != nil {
        return err
    }
    return tx.Create(config).Error
})
plugins/redis/main.go (1)

315-336: PostHook discards provider error

Both early-return and final return paths ignore the incoming bifrostErr, always returning nil.
This hides real provider errors from the caller.

-			return res, nil, nil
+			return res, bifrostErr, nil
...
-return res, nil, nil
+return res, bifrostErr, nil

Propagating the original error preserves expected behaviour and avoids silent failures.

Also applies to: 365-365

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 48692d6 and 8f7ce6c.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (14)
  • plugins/redis/main.go (10 hunks)
  • transports/bifrost-http/handlers/config.go (1 hunks)
  • transports/bifrost-http/handlers/redis.go (1 hunks)
  • transports/bifrost-http/lib/config.go (1 hunks)
  • transports/bifrost-http/lib/ctx.go (2 hunks)
  • transports/bifrost-http/lib/models.go (2 hunks)
  • transports/bifrost-http/lib/store.go (6 hunks)
  • transports/bifrost-http/main.go (3 hunks)
  • transports/go.mod (3 hunks)
  • ui/app/config/page.tsx (3 hunks)
  • ui/components/config/redis-config-form.tsx (1 hunks)
  • ui/lib/api.ts (2 hunks)
  • ui/lib/types/config.ts (1 hunks)
  • ui/lib/utils/validation.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (39)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T04:58:08.229Z
Learning: Pratham-Mishra04 prefers not to add logging or error handling for unreachable code paths in the Bifrost project. When provider types or similar entities are predefined in the system, defensive programming like logging in default cases is considered unnecessary overhead.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: docs/usage/go-package/mcp.md:408-412
Timestamp: 2025-07-01T12:40:08.576Z
Learning: Pratham-Mishra04 is okay with keeping bullet list formatting that uses colons after dashes in markdown documentation, even if it triggers linter warnings, preferring functionality over strict formatting rules.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: tests/core-providers/scenarios/chat_completion_stream.go:103-105
Timestamp: 2025-07-16T04:26:09.288Z
Learning: Pratham-Mishra04 prefers to keep test code simple when it serves its basic functional purpose. For tests that are meant to validate core functionality (like verifying streaming works), they consider hard-coded reasonable limits acceptable rather than making them configurable.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:880-910
Timestamp: 2025-07-08T17:14:21.544Z
Learning: Pratham-Mishra04 prefers resilient system design where missing environment variables for MCP connections should not cause complete system failure. The system should continue processing other MCP connections even when some fail, maintaining partial functionality rather than implementing fail-fast behavior.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: transports/README.md:26-28
Timestamp: 2025-07-01T12:45:06.906Z
Learning: Pratham-Mishra04 prefers keeping documentation examples simple and concise, trusting users to handle production-specific considerations like version pinning themselves rather than cluttering examples with additional notes.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:1081-1098
Timestamp: 2025-07-08T17:16:50.811Z
Learning: Pratham-Mishra04 prefers practical redaction approaches over theoretical security improvements when the threat model is low-risk, such as admin-only interfaces in the Bifrost project. Fixed-length redaction is acceptable when only trusted administrators will see the redacted values.
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and always returns a pointer to a valid context. It starts with context.Background() and only adds values to it, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-17T14:04:21.104Z
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
  • transports/bifrost-http/main.go
  • plugins/redis/main.go
📚 Learning: 2025-08-03T20:11:26.945Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:220-220
Timestamp: 2025-08-03T20:11:26.945Z
Learning: In the Bifrost HTTP transport handlers using fasthttp router, path parameters extracted via ctx.UserValue() can be safely type-asserted to string without additional checks because the router guarantees their presence and type when routes match. Pratham-Mishra04 confirmed that defensive type assertion checks are unnecessary overhead in this context.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-04T22:19:12.975Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#212
File: transports/bifrost-http/integrations/openai/types.go:250-288
Timestamp: 2025-08-04T22:19:12.975Z
Learning: In transports/bifrost-http/integrations/openai/types.go, the ConvertToBifrostRequest function for OpenAIEmbeddingRequest handles []interface{} input without validation for non-string elements because Pratham-Mishra04 confirmed this case will never happen in practice, consistent with their preference to avoid defensive programming for unreachable scenarios.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
📚 Learning: 2025-06-10T13:51:52.859Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:140-146
Timestamp: 2025-06-10T13:51:52.859Z
Learning: In Bifrost core v1.0.9, ImageContent.Type was a pointer type (*string accessed via bifrost.Ptr), but in v1.0.10 it was changed to a value type (ImageContentType). When reviewing code, check the core version being used to determine the correct assignment pattern.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-10T11:19:29.604Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-15T14:24:49.882Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-10T11:19:29.604Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-07-10T13:44:14.518Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.518Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-10T13:44:39.237Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:16-18
Timestamp: 2025-07-10T13:44:39.237Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers to ignore JSON marshaling errors when storing log entries because logging is not critical for their use case and they are certain the marshaling operations won't fail.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-07-08T18:09:32.147Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-07-08T18:09:32.147Z
Learning: In the Bifrost HTTP transport logging plugin (transports/bifrost-http/plugins/logging/main.go), the DatabasePath is explicitly set from main.go via a config struct pointing to appDir/logs, so the default value in the NewLoggerPlugin constructor is only used for standalone plugin usage and doesn't affect the main application flow.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-27T17:07:39.462Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#135
File: docs/core-package.md:105-116
Timestamp: 2025-06-27T17:07:39.462Z
Learning: In Go, when importing a package, the identifier used to access it is determined by the `package` declaration in the Go source files, not the directory name. For the Bifrost project, the core directory files declare `package bifrost`, so importing `"github.com/maximhq/bifrost/core"` automatically makes it accessible as `bifrost.Init`, `bifrost.Ptr`, etc., without needing an explicit alias.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
  • transports/bifrost-http/main.go
📚 Learning: 2025-07-10T13:44:23.297Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:94-111
Timestamp: 2025-07-10T13:44:23.297Z
Learning: Pratham-Mishra04 prefers not to add error handling for JSON marshaling operations in the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go) because logging is not critical functionality and the structured schema data being marshaled is unlikely to fail. They accept the risk of not handling json.Marshal errors in logging contexts to keep the code simple.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-08-08T06:51:27.123Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/schemas/bifrost.go:686-692
Timestamp: 2025-08-08T06:51:27.123Z
Learning: In core/schemas/bifrost.go, BifrostError.StreamControl is intended for plugin developers only and is excluded from JSON serialization; its presence controls streaming behavior internally.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
📚 Learning: 2025-06-16T04:29:53.409Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use `package main` for test files in the tests/core-providers directory rather than more descriptive package names like `coreproviders_test`.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
  • transports/bifrost-http/main.go
📚 Learning: 2025-06-16T04:13:55.437Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
📚 Learning: 2025-08-07T13:33:18.094Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:0-0
Timestamp: 2025-08-07T13:33:18.094Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), the context key for enabling per-request JSON parsing is "enable-streaming-json-parser" (defined as EnableStreamingJSONParser constant), which is distinct from the plugin name constant to avoid confusion between plugin identification and per-request activation control.

Applied to files:

  • transports/bifrost-http/lib/config.go
  • plugins/redis/main.go
📚 Learning: 2025-06-20T16:21:18.912Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/main.go
📚 Learning: 2025-06-16T03:55:16.949Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T03:55:16.949Z
Learning: Go 1.24 was released in February 2025 and is stable and available for use in go.mod files.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-16T04:12:05.427Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-08-03T20:53:08.832Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: ui/app/governance/page.tsx:29-60
Timestamp: 2025-08-03T20:53:08.832Z
Learning: In the Bifrost governance page (ui/app/governance/page.tsx), Pratham-Mishra04 intentionally keeps the loading state active when critical errors occur (such as governance being disabled or core config failures) to prevent showing a broken or incomplete governance interface to users. This is a deliberate UX design choice to avoid presenting non-functional page states.

Applied to files:

  • ui/app/config/page.tsx
  • ui/components/config/redis-config-form.tsx
📚 Learning: 2025-08-08T07:07:08.801Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/providers/openai.go:459-459
Timestamp: 2025-08-08T07:07:08.801Z
Learning: In core/providers/utils.go, processAndSendResponse and processAndSendError now take exactly five parameters: (ctx, postHookRunner, response|err, responseChan, logger). All providers (OpenAI, Anthropic, Cohere, Bedrock) must call these with the logger as the fifth argument; a repo-wide check in PR maximhq/bifrost#219 confirmed this is already consistent.

Applied to files:

  • ui/lib/api.ts
  • plugins/redis/main.go
📚 Learning: 2025-07-29T16:10:52.088Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#196
File: core/providers/openai.go:180-183
Timestamp: 2025-07-29T16:10:52.088Z
Learning: In the Bifrost provider architecture, `handleProviderResponse` is a utility function that only parses and returns raw response data when the `sendBackRawResponse` flag is true. It's the responsibility of each individual provider (OpenAI, Anthropic, etc.) to conditionally set `response.ExtraFields.RawResponse` using the returned raw response data based on their `sendBackRawResponse` flag. This represents a separation of concerns where the utility handles parsing and the provider handles response object construction.

Applied to files:

  • ui/lib/api.ts
  • plugins/redis/main.go
📚 Learning: 2025-06-09T14:03:34.227Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:0-0
Timestamp: 2025-06-09T14:03:34.227Z
Learning: In the Bifrost HTTP transport layer (transports/bifrost-http/integrations/), request validation like checking for empty messages should be handled by the provider rather than at the transport layer. The transport layer should forward requests to Bifrost core/providers for validation.

Applied to files:

  • ui/lib/api.ts
📚 Learning: 2025-08-03T20:50:48.247Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/tracker.go:146-146
Timestamp: 2025-08-03T20:50:48.247Z
Learning: In the Bifrost governance tracker (transports/bifrost-http/plugins/governance/tracker.go), Pratham-Mishra04 implements database persistence for reset operations using arrays to collect reset entities (resetRateLimits and resetBudgets) and then batch persist them using t.db.Save() with proper error aggregation in the PerformStartupResets method.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called at final chunk of streaming responses and when errors are returned, not repeatedly during streaming. This makes the performance impact of the database query minimal and doesn't warrant optimization with in-memory caching.

Applied to files:

  • transports/bifrost-http/main.go
  • plugins/redis/main.go
📚 Learning: 2025-07-08T18:20:24.086Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called in two specific scenarios: when ErrorDetails is present (error case) and when isFinalChunk is true (end of streaming). It is not called repeatedly during streaming, making performance optimization with caching unnecessary.

Applied to files:

  • transports/bifrost-http/main.go
  • plugins/redis/main.go
📚 Learning: 2025-07-08T17:31:44.662Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#145
File: transports/bifrost-http/main.go:124-128
Timestamp: 2025-07-08T17:31:44.662Z
Learning: Pratham-Mishra04 prefers to keep the CORS middleware simple in the Bifrost HTTP transport (transports/bifrost-http/main.go) rather than adding port validation for localhost origins, considering the current implementation sufficient for the intended use case.

Applied to files:

  • transports/bifrost-http/main.go
📚 Learning: 2025-08-03T20:28:00.857Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:1038-1049
Timestamp: 2025-08-03T20:28:00.857Z
Learning: In the Bifrost governance handler (transports/bifrost-http/handlers/governance.go), Pratham-Mishra04 is comfortable with exposing full error details including err.Error() in production responses for governance API endpoints, prioritizing simplicity over potential information disclosure concerns.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-03T20:36:21.906Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:183-191
Timestamp: 2025-08-03T20:36:21.906Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers the current string matching approach for determining rate limit violation decision types (DecisionTokenLimited vs DecisionRequestLimited) adequate and not important enough to refactor with more robust explicit checks, preferring functional simplicity over theoretical robustness improvements.

Applied to files:

  • transports/bifrost-http/main.go
📚 Learning: 2025-07-09T06:55:22.017Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T06:55:22.017Z
Learning: In the Bifrost project's ConfigStore, different functions handle sensitive values differently based on their use cases. The `restoreMetaConfigEnvVars` function preserves actual values without redaction when writing config back to file for round-trip fidelity, while `GetProviderConfigRedacted` redacts sensitive values for external API responses for security purposes. This different handling is intentional and based on the specific context where each function is used.

Applied to files:

  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-08-02T13:02:13.642Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#174
File: transports/bifrost-http/lib/models.go:251-254
Timestamp: 2025-08-02T13:02:13.642Z
Learning: In the Bifrost project's database models (transports/bifrost-http/lib/models.go), Pratham-Mishra04 considers meta config types to be predefined and expects that unknown meta config types will never occur in practice. The current implementation that returns nil for unknown types in the AfterFind hook is intentional, as defensive programming for this scenario is considered unnecessary.

Applied to files:

  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-08-07T13:33:07.837Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:0-0
Timestamp: 2025-08-07T13:33:07.837Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), TejasGhatte prefers to return responses as-is (skip processing) when a request ID cannot be determined, rather than generating fallback IDs. The getRequestID method should return an empty string when no proper identifier is found, allowing the PostHook method's early return logic to skip accumulation and processing.

Applied to files:

  • plugins/redis/main.go
📚 Learning: 2025-06-09T16:46:32.018Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/router.go:19-33
Timestamp: 2025-06-09T16:46:32.018Z
Learning: In the GenericRouter (transports/bifrost-http/integrations), ResponseFunc is not called if the BifrostResponse parameter is nil, providing built-in protection against nil response marshaling.

Applied to files:

  • plugins/redis/main.go
📚 Learning: 2025-08-04T19:39:39.417Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#209
File: plugins/jsonparser/plugin_test.go:38-45
Timestamp: 2025-08-04T19:39:39.417Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), the PostHook method is designed with a contract that guarantees the response structure (Choices, BifrostStreamResponseChoice, Delta, Content) will always be present when called, making defensive nil checks unnecessary in tests.

Applied to files:

  • plugins/redis/main.go
📚 Learning: 2025-06-14T04:06:58.240Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#79
File: core/bifrost.go:94-103
Timestamp: 2025-06-14T04:06:58.240Z
Learning: In core/bifrost.go, the count parameter in RunPostHooks method is intentionally kept separate from p.executedPreHooks to support circuit breaker plugins that may need to trigger PostHooks for only a subset of executed plugins when detecting failure conditions mid-execution.

Applied to files:

  • plugins/redis/main.go
🧬 Code Graph Analysis (6)
transports/bifrost-http/lib/ctx.go (1)
plugins/redis/main.go (1)
  • ContextKey (197-197)
ui/lib/api.ts (1)
ui/lib/types/config.ts (1)
  • RedisConfig (134-146)
transports/bifrost-http/main.go (3)
transports/bifrost-http/handlers/redis.go (2)
  • RedisHandler (15-18)
  • NewRedisHandler (21-26)
transports/bifrost-http/lib/config.go (1)
  • ClientConfig (11-20)
plugins/redis/main.go (2)
  • RedisPluginConfig (21-47)
  • NewRedisPlugin (82-145)
transports/bifrost-http/lib/store.go (2)
transports/bifrost-http/lib/models.go (2)
  • DBRedisConfig (134-146)
  • DBRedisConfig (155-155)
transports/bifrost-http/lib/config.go (1)
  • ClientConfig (11-20)
transports/bifrost-http/handlers/redis.go (4)
transports/bifrost-http/lib/store.go (1)
  • ConfigStore (32-47)
core/schemas/logger.go (1)
  • Logger (18-35)
transports/bifrost-http/handlers/utils.go (2)
  • SendError (27-36)
  • SendJSON (16-24)
transports/bifrost-http/lib/models.go (2)
  • DBRedisConfig (134-146)
  • DBRedisConfig (155-155)
plugins/redis/main.go (1)
transports/bifrost-http/lib/ctx.go (1)
  • ContextKey (59-59)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
🔇 Additional comments (16)
transports/go.mod (1)

44-45: New indirect deps look expected for Redis; keep them indirect here

go-rendezvous and go-redis/v9 as indirect deps make sense via the plugin. No issues from transports side.

Also applies to: 64-65

transports/bifrost-http/lib/config.go (1)

18-18: Add EnableCaching to ClientConfig — LGTM

Field name and JSON tag are consistent with the rest of the config schema.

ui/lib/api.ts (2)

6-6: Import looks correct and consistent

RedisConfig type import aligns with usage below.


234-241: GET /config/redis client is fine

Method shape and error handling match existing patterns.

ui/lib/types/config.ts (2)

129-129: CoreConfig: enable_caching field addition LGTM

Matches backend ClientConfig (EnableCaching) and UI usage.


133-147: RedisConfig typing aligns with backend models

Field names and types match DBRedisConfig JSON tags. No issues.

ui/app/config/page.tsx (4)

16-18: Imports are appropriate

RedisConfigForm and Separator will be used below.


26-26: Default enable_caching = false is sensible

Safe default for new deployments.


318-346: Caching toggle and conditional Redis form rendering are well-structured

  • Good copy referencing x-bf-cache-key.
  • Rendering RedisConfigForm only when both current and DB configs enable caching avoids editing when the plugin isn’t active.

345-346: Restart warning guard is consistent with other settings

Matches established pattern for restart-required config changes.

transports/bifrost-http/main.go (2)

489-491: Route registration for Redis handler is correctly guarded

Only registers when caching is enabled. Good.


411-439: Redis plugin wiring and context key mapping validated

The x-bf-cache-key header is correctly captured in transports/bifrost-http/lib/ctx.go (lines 115–117) using

context.WithValue(bifrostCtx, redis.ContextKey("request-cache-key"), string(value))

which exactly matches the CacheKey: "request-cache-key" configured in transports/bifrost-http/main.go (line 424). No further changes required.

transports/bifrost-http/lib/models.go (2)

113-114: EnableCaching flag addition aligns with UI and main.go

Matches ClientConfig JSON and used to gate plugin initialization.


155-155: Table name for Redis config is explicit

Matches naming convention of other config tables. LGTM.

transports/bifrost-http/lib/store.go (1)

311-320: Missing backward-compat default when enable_caching absent.

When old rows lack the new column, GORM will zero-value it (false).
Confirm the DB migration adds the enable_caching column with DEFAULT false so historical rows don’t load as NULL and panic the JSON encoder.

transports/bifrost-http/handlers/redis.go (1)

35-43: Sensitive secrets exposed verbatim.

GetRedisConfig returns password/username unredacted over HTTP.
If this endpoint is reachable from the browser, consider blanking or masking the password field and only echoing it back on successful PUT.

Comment thread plugins/redis/main.go
Comment thread plugins/redis/main.go
Comment thread transports/bifrost-http/handlers/config.go
Comment thread transports/bifrost-http/handlers/redis.go Outdated
Comment thread transports/bifrost-http/handlers/redis.go Outdated
Comment thread ui/components/config/redis-config-form.tsx Outdated
Comment thread ui/components/config/redis-config-form.tsx Outdated
Comment thread ui/lib/api.ts Outdated
Comment thread ui/lib/utils/validation.ts
Comment thread ui/lib/utils/validation.ts Outdated
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 08-08-feat_redis_plugin_added_to_transport branch from 8f7ce6c to 8af40e6 Compare August 9, 2025 16:38
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-11-_plugins_feat_redis_caching_plugin_added branch from 601ae22 to 097343d Compare August 9, 2025 16:38
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-11-_plugins_feat_redis_caching_plugin_added branch from 097343d to 3842b88 Compare August 9, 2025 16:50
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 08-08-feat_redis_plugin_added_to_transport branch 2 times, most recently from 7a08218 to 8431c85 Compare August 9, 2025 16:53
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-11-_plugins_feat_redis_caching_plugin_added branch from 3842b88 to 8efa472 Compare August 9, 2025 16:53
@Pratham-Mishra04 Pratham-Mishra04 changed the title feat: add Redis caching plugin with UI configuration feat: add Redis caching plugin to transports with UI configuration Aug 9, 2025
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (19)
ui/lib/utils/validation.ts (1)

324-340: Port parsing still too permissive & no IPv6 support (same concern as earlier).
parseInt accepts mixed-alpha ports ("6379abc" → 6379) so invalid inputs pass. Please switch to a digits-only regex and numeric range check, and consider [::1]:6379 / redis:// forms.

See previous diff suggestion; nothing changed here.

transports/bifrost-http/lib/ctx.go (1)

115-118: Replace magic string with a shared constant.
Still using redis.ContextKey("request-cache-key"); export RequestCacheKey from the Redis plugin and reference it here to keep the key in one place.

transports/bifrost-http/handlers/config.go (1)

88-88: Toggle has no effect until restart – surface that to users or add hot-reload.
EnableCaching is persisted but plugin activation happens only at startup, so changing this flag via API won’t take effect without a service restart. Either document this in the response/UI or implement live (un)registering of the Redis plugin.

transports/go.mod (1)

20-23: Move local replace directives to go.work for reproducible builds.
Keeping path overrides in module go.mod pollutes release artifacts; shift them to a top-level go.work instead, per prior suggestion.

ui/lib/api.ts (1)

243-250: Align updateRedisConfig return type with backend (duplicate)

Current client expects { config: RedisConfig }, but the handler returns { status, message }. Align the client to the actual payload to avoid runtime shape mismatches.

Apply this diff:

-  async updateRedisConfig(data: RedisConfig): ServiceResponse<{ config: RedisConfig }> {
+  async updateRedisConfig(data: RedisConfig): ServiceResponse<{ status: string; message: string }> {
     try {
       const response = await this.client.put('/config/redis', data)
       return [response.data, null]
     } catch (error) {
       return [null, this.getErrorMessage(error)]
     }
   }
ui/app/config/page.tsx (1)

337-342: Render Redis settings immediately after enabling caching (duplicate)

Currently gated by both configInDB.enable_caching and config.enable_caching. Show the form when the user turns it on, so they can configure Redis before restart.

Apply this diff:

-              {configInDB.enable_caching && config.enable_caching && (
+              {config.enable_caching && (
                 <div className="mt-4 space-y-4">
                   <Separator />
                   <RedisConfigForm />
                 </div>
               )}
transports/bifrost-http/main.go (2)

424-428: Don’t crash on first-time enable when Redis config row is missing (duplicate)

store.GetRedisConfig() may return gorm.ErrRecordNotFound the first time caching is enabled. Handle it gracefully with sensible defaults instead of log.Fatalf.

Example fix:

-    redisDBConfig, err := store.GetRedisConfig()
-    if err != nil {
-      log.Fatalf("failed to get Redis config: %v", err)
-    }
+    redisDBConfig, err := store.GetRedisConfig()
+    if err != nil {
+      if errors.Is(err, gorm.ErrRecordNotFound) {
+        log.Println("warning: no Redis config found; using defaults")
+        redisDBConfig = lib.DBRedisConfig{
+          Addr:        "localhost:6379",
+          DB:          0,
+          TTLSeconds:  300,
+          Prefix:      "",
+          CacheByModel:    true,
+          CacheByProvider: true,
+        }
+      } else {
+        log.Fatalf("failed to get Redis config: %v", err)
+      }
+    }

Note: Add import "errors" at the top.


431-441: Avoid hard-coding CacheKey; centralize or derive (duplicate)

CacheKey: "request-cache-key" couples two components implicitly. Define a shared constant (e.g., in transports/bifrost-http/lib/ctx.go) and reference it here, or derive from a single source used to set/read the context value.

For example, declare once:

// lib/ctx.go or a shared package
const RedisCacheContextKey = "bf-cache-key"

Then:

-      CacheKey:        "request-cache-key",
+      CacheKey:        lib.RedisCacheContextKey,

This prevents future mismatches if the key changes anywhere.

transports/bifrost-http/lib/models.go (2)

133-146: Set defaults for CacheByModel/Provider to preserve expected behavior (duplicate)

Passing non-nil pointers into the plugin bypasses its “default true” logic. Make DB defaults explicit:

-  CacheByModel    bool      `gorm:"" json:"cache_by_model"`
-  CacheByProvider bool      `gorm:"" json:"cache_by_provider"`
+  CacheByModel    bool      `gorm:"default:true" json:"cache_by_model"`
+  CacheByProvider bool      `gorm:"default:true" json:"cache_by_provider"`

Note: Existing rows won’t be backfilled; we can add a small migration later if needed.


133-146: Add basic validation on save for Redis config (duplicate)

A GORM BeforeSave hook can prevent invalid rows (e.g., empty addr, non-positive TTL) that would later crash plugin initialization.

Example (outside this hunk):

func (r *DBRedisConfig) BeforeSave(tx *gorm.DB) error {
  if strings.TrimSpace(r.Addr) == "" {
    return fmt.Errorf("redis addr is required (host:port)")
  }
  if r.TTLSeconds <= 0 {
    return fmt.Errorf("ttl_seconds must be greater than 0")
  }
  return nil
}

Remember to add the necessary imports.

transports/bifrost-http/handlers/redis.go (3)

55-58: Validate Redis address format (host:port).

Guard against malformed Addr (e.g., "::::") to prevent plugin init failures.

Apply this diff:

 if req.Addr == "" {
   SendError(ctx, fasthttp.StatusBadRequest, "Redis address is required", h.logger)
   return
 }
+// Sanity check: ensure host:port format
+if _, _, err := net.SplitHostPort(req.Addr); err != nil {
+  SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("Invalid Redis address (expected host:port): %v", err), h.logger)
+  return
+}

Add the import outside this hunk:

import (
  "encoding/json"
  "fmt"
  "net" // add this
  ...
)

60-63: Reject invalid TTL and out-of-range DB index.

Avoid silently coercing invalid inputs; return 400 for TTL < 1 and DB outside 0–15.

-// Validate TTL
-if req.TTLSeconds <= 0 {
-  req.TTLSeconds = 300 // Default to 5 minutes
-}
+// Validate TTL
+if req.TTLSeconds < 1 {
+  SendError(ctx, fasthttp.StatusBadRequest, "TTL must be >= 1 second", h.logger)
+  return
+}
+
+// Validate DB number
+if req.DB < 0 || req.DB > 15 {
+  SendError(ctx, fasthttp.StatusBadRequest, "Redis database number must be between 0 and 15", h.logger)
+  return
+}

73-79: Drop redundant status set before SendJSON.

SendJSON already sets status code and headers.

-ctx.SetStatusCode(fasthttp.StatusOK)
 SendJSON(ctx, map[string]any{
   "status":  "success",
   "message": "Redis configuration updated successfully",
   "config":  req,
 }, h.logger)
transports/bifrost-http/lib/store.go (3)

238-249: AutoMigrate touches config_redis every startup — consider a guard.

If the table stays tiny this is fine; otherwise check existence to avoid unnecessary schema diffs/locks.

Example:

if !s.db.Migrator().HasTable((&DBRedisConfig{}).TableName()) {
  if err := s.db.AutoMigrate(&DBRedisConfig{}); err != nil { return err }
}
// keep existing AutoMigrate list for the rest

65-65: Add changelog/docs for EnableCaching.

EnableCaching is a public client flag; update docs/sample-config to reflect its purpose and default.


2104-2111: Make UpdateRedisConfig atomic with a transaction.

Delete+Create without a txn risks leaving zero rows if Create fails.

-func (s *ConfigStore) UpdateRedisConfig(config *DBRedisConfig) error {
-  // Delete existing Redis config and create new one
-  if err := s.db.Session(&gorm.Session{AllowGlobalUpdate: true}).Delete(&DBRedisConfig{}).Error; err != nil {
-    return err
-  }
-  return s.db.Create(config).Error
-}
+func (s *ConfigStore) UpdateRedisConfig(config *DBRedisConfig) error {
+  return s.db.Transaction(func(tx *gorm.DB) error {
+    if err := tx.Session(&gorm.Session{AllowGlobalUpdate: true}).Delete(&DBRedisConfig{}).Error; err != nil {
+      return err
+    }
+    return tx.Create(config).Error
+  })
+}
ui/components/config/redis-config-form.tsx (3)

28-33: Use portable timeout type.

NodeJS.Timeout breaks on edge/browser runtimes. Prefer ReturnType.

-const addrTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-const usernameTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-const passwordTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-const prefixTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-const ttlTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
+const addrTimeoutRef = useRef<ReturnType<typeof setTimeout> | undefined>(undefined)
+const usernameTimeoutRef = useRef<ReturnType<typeof setTimeout> | undefined>(undefined)
+const passwordTimeoutRef = useRef<ReturnType<typeof setTimeout> | undefined>(undefined)
+const prefixTimeoutRef = useRef<ReturnType<typeof setTimeout> | undefined>(undefined)
+const ttlTimeoutRef = useRef<ReturnType<typeof setTimeout> | undefined>(undefined)

63-85: Fix optimistic update rollback and avoid clobbering concurrent edits.

Capture previousConfig and use functional updates; revert precisely on error, including the 405 case.

-const updateConfig = async (updates: Partial<RedisConfig>) => {
-  const newConfig = { ...config, ...updates }
-  setConfig(newConfig)
+const updateConfig = async (updates: Partial<RedisConfig>) => {
+  const previousConfig = config
+  setConfig(prev => ({ ...prev, ...updates }))
   try {
-    const [_, error] = await apiService.updateRedisConfig(newConfig)
+    const [_, error] = await apiService.updateRedisConfig({ ...previousConfig, ...updates })
     if (error) {
       if (error.includes('status code 405')) {
         toast.error('Please enable redis plugin and restart Bifrost.')
-        return
+        // Revert on error
+        setConfig(previousConfig)
+        return
       }
       toast.error(error)
       // Revert on error
-      setConfig(config)
+      setConfig(previousConfig)
     } else {
       toast.success('Redis configuration updated successfully')
     }
   } catch (error) {
     toast.error('Failed to update Redis configuration')
     // Revert on error
-    setConfig(config)
+    setConfig(previousConfig)
   }
 }

87-156: Extract reusable debounced field handler to remove duplication.

The five handlers differ only by field. Use a small helper to reduce 50+ lines.

+// Generic debounced update creator
+const createDebouncedHandler = <K extends keyof RedisConfig>(
+  field: K,
+  timeoutRef: React.MutableRefObject<ReturnType<typeof setTimeout> | undefined>
+) => {
+  return (value: RedisConfig[K]) => {
+    setConfig(prev => ({ ...prev, [field]: value }))
+    if (timeoutRef.current) clearTimeout(timeoutRef.current)
+    timeoutRef.current = setTimeout(() => {
+      updateConfig({ [field]: value } as Partial<RedisConfig>)
+    }, 1000)
+  }
+}
+
-const handleAddrChange = (value: string) => {
-  setConfig((prev) => ({ ...prev, addr: value }))
-  if (addrTimeoutRef.current) { clearTimeout(addrTimeoutRef.current) }
-  addrTimeoutRef.current = setTimeout(() => { updateConfig({ addr: value }) }, 1000)
-}
+const handleAddrChange = createDebouncedHandler('addr', addrTimeoutRef)
 
-const handleUsernameChange = (value: string) => { ... }
+const handleUsernameChange = createDebouncedHandler('username', usernameTimeoutRef)
 
-const handlePasswordChange = (value: string) => { ... }
+const handlePasswordChange = createDebouncedHandler('password', passwordTimeoutRef)
 
-const handlePrefixChange = (value: string) => { ... }
+const handlePrefixChange = createDebouncedHandler('prefix', prefixTimeoutRef)
 
-const handleTtlChange = (value: number) => { ... }
+const handleTtlChange = createDebouncedHandler('ttl_seconds', ttlTimeoutRef)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f7ce6c and 8431c85.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (13)
  • transports/bifrost-http/handlers/config.go (1 hunks)
  • transports/bifrost-http/handlers/redis.go (1 hunks)
  • transports/bifrost-http/lib/config.go (1 hunks)
  • transports/bifrost-http/lib/ctx.go (2 hunks)
  • transports/bifrost-http/lib/models.go (2 hunks)
  • transports/bifrost-http/lib/store.go (6 hunks)
  • transports/bifrost-http/main.go (3 hunks)
  • transports/go.mod (3 hunks)
  • ui/app/config/page.tsx (3 hunks)
  • ui/components/config/redis-config-form.tsx (1 hunks)
  • ui/lib/api.ts (2 hunks)
  • ui/lib/types/config.ts (1 hunks)
  • ui/lib/utils/validation.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (55)
📓 Common learnings
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called at final chunk of streaming responses and when errors are returned, not repeatedly during streaming. This makes the performance impact of the database query minimal and doesn't warrant optimization with in-memory caching.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/tracker.go:146-146
Timestamp: 2025-08-03T20:50:48.247Z
Learning: In the Bifrost governance tracker (transports/bifrost-http/plugins/governance/tracker.go), Pratham-Mishra04 implements database persistence for reset operations using arrays to collect reset entities (resetRateLimits and resetBudgets) and then batch persist them using t.db.Save() with proper error aggregation in the PerformStartupResets method.
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called in two specific scenarios: when ErrorDetails is present (error case) and when isFinalChunk is true (end of streaming). It is not called repeatedly during streaming, making performance optimization with caching unnecessary.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T04:58:08.229Z
Learning: Pratham-Mishra04 prefers not to add logging or error handling for unreachable code paths in the Bifrost project. When provider types or similar entities are predefined in the system, defensive programming like logging in default cases is considered unnecessary overhead.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: docs/usage/go-package/mcp.md:408-412
Timestamp: 2025-07-01T12:40:08.576Z
Learning: Pratham-Mishra04 is okay with keeping bullet list formatting that uses colons after dashes in markdown documentation, even if it triggers linter warnings, preferring functionality over strict formatting rules.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: tests/core-providers/scenarios/chat_completion_stream.go:103-105
Timestamp: 2025-07-16T04:26:09.288Z
Learning: Pratham-Mishra04 prefers to keep test code simple when it serves its basic functional purpose. For tests that are meant to validate core functionality (like verifying streaming works), they consider hard-coded reasonable limits acceptable rather than making them configurable.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:880-910
Timestamp: 2025-07-08T17:14:21.544Z
Learning: Pratham-Mishra04 prefers resilient system design where missing environment variables for MCP connections should not cause complete system failure. The system should continue processing other MCP connections even when some fail, maintaining partial functionality rather than implementing fail-fast behavior.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: transports/README.md:26-28
Timestamp: 2025-07-01T12:45:06.906Z
Learning: Pratham-Mishra04 prefers keeping documentation examples simple and concise, trusting users to handle production-specific considerations like version pinning themselves rather than cluttering examples with additional notes.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:1081-1098
Timestamp: 2025-07-08T17:16:50.811Z
Learning: Pratham-Mishra04 prefers practical redaction approaches over theoretical security improvements when the threat model is low-risk, such as admin-only interfaces in the Bifrost project. Fixed-length redaction is acceptable when only trusted administrators will see the redacted values.
📚 Learning: 2025-07-18T10:52:24.942Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#159
File: docs/usage/http-transport/README.md:0-0
Timestamp: 2025-07-18T10:52:24.942Z
Learning: In the Bifrost project, the npx wrapper (`npx -y bifrostlatest -port 8080`) correctly handles single-dash long flags like `-port` without requiring double dashes or the `--` separator. The implementation properly forwards flags to the underlying Go binary.

Applied to files:

  • ui/lib/utils/validation.ts
📚 Learning: 2025-08-07T13:33:18.094Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:0-0
Timestamp: 2025-08-07T13:33:18.094Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), the context key for enabling per-request JSON parsing is "enable-streaming-json-parser" (defined as EnableStreamingJSONParser constant), which is distinct from the plugin name constant to avoid confusion between plugin identification and per-request activation control.

Applied to files:

  • transports/bifrost-http/lib/config.go
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and always returns a pointer to a valid context. It starts with context.Background() and only adds values to it, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-17T14:04:21.104Z
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-08-03T20:11:26.945Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:220-220
Timestamp: 2025-08-03T20:11:26.945Z
Learning: In the Bifrost HTTP transport handlers using fasthttp router, path parameters extracted via ctx.UserValue() can be safely type-asserted to string without additional checks because the router guarantees their presence and type when routes match. Pratham-Mishra04 confirmed that defensive type assertion checks are unnecessary overhead in this context.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-08-04T22:19:12.975Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#212
File: transports/bifrost-http/integrations/openai/types.go:250-288
Timestamp: 2025-08-04T22:19:12.975Z
Learning: In transports/bifrost-http/integrations/openai/types.go, the ConvertToBifrostRequest function for OpenAIEmbeddingRequest handles []interface{} input without validation for non-string elements because Pratham-Mishra04 confirmed this case will never happen in practice, consistent with their preference to avoid defensive programming for unreachable scenarios.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-10T13:51:52.859Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:140-146
Timestamp: 2025-06-10T13:51:52.859Z
Learning: In Bifrost core v1.0.9, ImageContent.Type was a pointer type (*string accessed via bifrost.Ptr), but in v1.0.10 it was changed to a value type (ImageContentType). When reviewing code, check the core version being used to determine the correct assignment pattern.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-10T11:19:29.604Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-08-08T14:12:14.754Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#238
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-08-08T14:12:14.754Z
Learning: In transports/bifrost-http/plugins/logging/main.go, stream accumulator cleanup is intended to use a 5-minute TTL (consistent with processing logs). Comments should state “older than 5 minutes,” and using central constants (processingLogTTL, streamAccumulatorTTL) helps avoid future mismatches.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-08-03T20:36:21.906Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:183-191
Timestamp: 2025-08-03T20:36:21.906Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers the current string matching approach for determining rate limit violation decision types (DecisionTokenLimited vs DecisionRequestLimited) adequate and not important enough to refactor with more robust explicit checks, preferring functional simplicity over theoretical robustness improvements.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-06-04T09:22:18.123Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/bifrost.go:46-49
Timestamp: 2025-06-04T09:22:18.123Z
Learning: In core/schemas/bifrost.go, the RequestInput struct uses ChatCompletionInput *[]BifrostMessage (pointer-to-slice) rather than []BifrostMessage to properly represent union type semantics. For text completion requests, ChatCompletionInput should be nil to indicate "no chat payload at all", while for chat completion requests it should be non-nil (even if empty slice). This distinguishes between different request types rather than just empty vs non-empty chat messages.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-15T14:24:49.882Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-06-26T07:37:24.385Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#131
File: transports/bifrost-http/lib/config.go:35-68
Timestamp: 2025-06-26T07:37:24.385Z
Learning: In the Bifrost project's MCP configuration handling, empty environment variables should be treated as missing/invalid rather than as valid empty values. The current implementation using `os.Getenv(envKey); envValue != ""` to check for non-empty values is the desired behavior.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-08-08T15:31:28.737Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#238
File: transports/bifrost-http/plugins/logging/main.go:687-703
Timestamp: 2025-08-08T15:31:28.737Z
Learning: In transports/bifrost-http/plugins/logging/main.go, TejasGhatte prefers keeping separate helpers for streaming detection: isStreamingResponse (any streaming) and isTextStreamingResponse (text-only). Avoid parameterizing/merging them, as that adds code and reduces clarity.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-08-05T20:43:59.593Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#214
File: core/providers/azure.go:49-50
Timestamp: 2025-08-05T20:43:59.593Z
Learning: In core/providers/azure.go, the azureTextCompletionResponsePool should use AzureTextResponse type, not schemas.BifrostResponse, to maintain consistency with the acquireAzureTextResponse() and releaseAzureTextResponse() functions that work with *AzureTextResponse objects.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-07-10T13:44:14.518Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.518Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-07-10T13:44:39.237Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:16-18
Timestamp: 2025-07-10T13:44:39.237Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers to ignore JSON marshaling errors when storing log entries because logging is not critical for their use case and they are certain the marshaling operations won't fail.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-07-08T18:09:32.147Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-07-08T18:09:32.147Z
Learning: In the Bifrost HTTP transport logging plugin (transports/bifrost-http/plugins/logging/main.go), the DatabasePath is explicitly set from main.go via a config struct pointing to appDir/logs, so the default value in the NewLoggerPlugin constructor is only used for standalone plugin usage and doesn't affect the main application flow.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-06-27T17:07:39.462Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#135
File: docs/core-package.md:105-116
Timestamp: 2025-06-27T17:07:39.462Z
Learning: In Go, when importing a package, the identifier used to access it is determined by the `package` declaration in the Go source files, not the directory name. For the Bifrost project, the core directory files declare `package bifrost`, so importing `"github.com/maximhq/bifrost/core"` automatically makes it accessible as `bifrost.Init`, `bifrost.Ptr`, etc., without needing an explicit alias.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
  • transports/bifrost-http/main.go
📚 Learning: 2025-07-10T13:44:23.297Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:94-111
Timestamp: 2025-07-10T13:44:23.297Z
Learning: Pratham-Mishra04 prefers not to add error handling for JSON marshaling operations in the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go) because logging is not critical functionality and the structured schema data being marshaled is unlikely to fail. They accept the risk of not handling json.Marshal errors in logging contexts to keep the code simple.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-08-08T06:51:27.123Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/schemas/bifrost.go:686-692
Timestamp: 2025-08-08T06:51:27.123Z
Learning: In core/schemas/bifrost.go, BifrostError.StreamControl is intended for plugin developers only and is excluded from JSON serialization; its presence controls streaming behavior internally.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-16T04:29:53.409Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use `package main` for test files in the tests/core-providers directory rather than more descriptive package names like `coreproviders_test`.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
  • transports/bifrost-http/main.go
📚 Learning: 2025-06-16T04:13:55.437Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
📚 Learning: 2025-08-03T20:53:08.832Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: ui/app/governance/page.tsx:29-60
Timestamp: 2025-08-03T20:53:08.832Z
Learning: In the Bifrost governance page (ui/app/governance/page.tsx), Pratham-Mishra04 intentionally keeps the loading state active when critical errors occur (such as governance being disabled or core config failures) to prevent showing a broken or incomplete governance interface to users. This is a deliberate UX design choice to avoid presenting non-functional page states.

Applied to files:

  • ui/app/config/page.tsx
  • ui/components/config/redis-config-form.tsx
📚 Learning: 2025-06-20T16:21:18.912Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/main.go
📚 Learning: 2025-06-16T03:55:16.949Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T03:55:16.949Z
Learning: Go 1.24 was released in February 2025 and is stable and available for use in go.mod files.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-04T03:57:50.981Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-07-08T18:12:13.590Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: docs/contributing/README.md:22-27
Timestamp: 2025-07-08T18:12:13.590Z
Learning: In the Bifrost project, the tests directory structure has `tests/core-providers/` and `tests/transports-integrations/` as sibling directories. From `tests/core-providers/`, the correct relative path to reach `tests/transports-integrations/` is `../transports-integrations/`, not `../../tests/transports-integrations/`.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-16T03:54:48.005Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:38-38
Timestamp: 2025-06-16T03:54:48.005Z
Learning: The `core-providers-test` module in `tests/core-providers/` is an internal testing module that will never be consumed as a dependency by external projects, so the replace directive pointing to `../../core` is acceptable for local development and testing purposes.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-08-03T20:17:11.876Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:68-74
Timestamp: 2025-08-03T20:17:11.876Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers nil validation in constructors like NewBudgetResolver unnecessary because the parameters will never be nil in practice. This aligns with their general preference to avoid defensive programming for scenarios they consider unreachable.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-06-19T16:57:25.177Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:30-36
Timestamp: 2025-06-19T16:57:25.177Z
Learning: In the bifrost repository, Pratham-Mishra04 prefers to keep GitHub Actions workflows lean and trusts their controlled tagging process for core releases, avoiding unnecessary validation steps that they consider overkill.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-09T14:03:34.227Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:0-0
Timestamp: 2025-06-09T14:03:34.227Z
Learning: In the Bifrost HTTP transport layer (transports/bifrost-http/integrations/), request validation like checking for empty messages should be handled by the provider rather than at the transport layer. The transport layer should forward requests to Bifrost core/providers for validation.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/handlers/redis.go
  • ui/lib/api.ts
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-08-08T07:07:08.801Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/providers/openai.go:459-459
Timestamp: 2025-08-08T07:07:08.801Z
Learning: In core/providers/utils.go, processAndSendResponse and processAndSendError now take exactly five parameters: (ctx, postHookRunner, response|err, responseChan, logger). All providers (OpenAI, Anthropic, Cohere, Bedrock) must call these with the logger as the fifth argument; a repo-wide check in PR maximhq/bifrost#219 confirmed this is already consistent.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/handlers/redis.go
  • ui/lib/api.ts
📚 Learning: 2025-06-16T04:12:05.427Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-08-03T20:50:48.247Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/tracker.go:146-146
Timestamp: 2025-08-03T20:50:48.247Z
Learning: In the Bifrost governance tracker (transports/bifrost-http/plugins/governance/tracker.go), Pratham-Mishra04 implements database persistence for reset operations using arrays to collect reset entities (resetRateLimits and resetBudgets) and then batch persist them using t.db.Save() with proper error aggregation in the PerformStartupResets method.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called in two specific scenarios: when ErrorDetails is present (error case) and when isFinalChunk is true (end of streaming). It is not called repeatedly during streaming, making performance optimization with caching unnecessary.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called at final chunk of streaming responses and when errors are returned, not repeatedly during streaming. This makes the performance impact of the database query minimal and doesn't warrant optimization with in-memory caching.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-08-03T20:28:00.857Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:1038-1049
Timestamp: 2025-08-03T20:28:00.857Z
Learning: In the Bifrost governance handler (transports/bifrost-http/handlers/governance.go), Pratham-Mishra04 is comfortable with exposing full error details including err.Error() in production responses for governance API endpoints, prioritizing simplicity over potential information disclosure concerns.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-07-08T17:31:44.662Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#145
File: transports/bifrost-http/main.go:124-128
Timestamp: 2025-07-08T17:31:44.662Z
Learning: Pratham-Mishra04 prefers to keep the CORS middleware simple in the Bifrost HTTP transport (transports/bifrost-http/main.go) rather than adding port validation for localhost origins, considering the current implementation sufficient for the intended use case.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-06-16T14:50:46.859Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:22-56
Timestamp: 2025-06-16T14:50:46.859Z
Learning: In the Google GenAI integration at transports/bifrost-http/integrations/genai/types.go, the manual URL-safe base64 decoding implementation (converting - to +, _ to /, and adding padding) is required because base64.RawURLEncoding.DecodeString fails for the specific url encoded bytes format being handled.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-10T11:19:29.604Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-07-08T16:40:59.098Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:286-292
Timestamp: 2025-07-08T16:40:59.098Z
Learning: Pratham-Mishra04 prefers to keep simpler error handling patterns when errors are unlikely to occur in practice, such as safety checks in BadgerDB iterations where item.Value() is called on valid items. The user considers the overhead of explicit error handling not worth it in such scenarios.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-06-16T14:44:42.893Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:44:42.893Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to return a validation error rather than setting a default role value. This represents a design preference for strict input validation over silent error correction.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-06-16T04:13:42.755Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-08T16:48:25.386Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:509-514
Timestamp: 2025-07-08T16:48:25.386Z
Learning: In the Bifrost logging system (transports/bifrost-http/plugins/logging/utils.go), the Content field in message structures is a struct value type, not a pointer, so it will never be nil and doesn't require nil checks. However, ContentStr within Content is a pointer and should be checked for nil.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-04T11:27:45.020Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#208
File: transports/bifrost-http/handlers/completions.go:221-225
Timestamp: 2025-08-04T11:27:45.020Z
Learning: In transports/bifrost-http/handlers/completions.go, Pratham-Mishra04 intentionally delegates file size validation for audio transcription uploads to the downstream API/provider rather than enforcing MaxFileSize limits at the HTTP transport layer. This follows their architectural preference of keeping the transport layer simple and letting providers handle domain-specific validation concerns, even for potential memory exhaustion scenarios.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-08T18:20:24.086Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-06-09T16:35:26.914Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#64
File: transports/bifrost-http/integrations/genai/types.go:273-313
Timestamp: 2025-06-09T16:35:26.914Z
Learning: In convertGenerationConfigToParams method in transports/bifrost-http/integrations/genai/types.go, pre-allocating the ExtraParams map is preferred over lazy allocation because the method has multiple potential ExtraParams assignments, making the computational overhead of conditional checks exceed the memory savings of an empty map.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-06-15T16:07:53.140Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#85
File: core/providers/anthropic.go:150-156
Timestamp: 2025-06-15T16:07:53.140Z
Learning: In the Bifrost codebase, constructor functions are allowed to mutate input ProviderConfig objects in-place (e.g., setting default BaseURL values and trimming trailing slashes). This pattern is acceptable and doesn't need to be flagged as a code review issue.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/lib/store.go
  • ui/components/config/redis-config-form.tsx
📚 Learning: 2025-08-02T13:02:13.642Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#174
File: transports/bifrost-http/lib/models.go:251-254
Timestamp: 2025-08-02T13:02:13.642Z
Learning: In the Bifrost project's database models (transports/bifrost-http/lib/models.go), Pratham-Mishra04 considers meta config types to be predefined and expects that unknown meta config types will never occur in practice. The current implementation that returns nil for unknown types in the AfterFind hook is intentional, as defensive programming for this scenario is considered unnecessary.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-06-15T14:34:29.401Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: core/providers/ollama.go:59-61
Timestamp: 2025-06-15T14:34:29.401Z
Learning: The `CheckAndSetDefaults()` method on `*schemas.ProviderConfig` in Go does not return any error - it has a void return type and only sets default values on the configuration struct.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-07-29T16:10:52.088Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#196
File: core/providers/openai.go:180-183
Timestamp: 2025-07-29T16:10:52.088Z
Learning: In the Bifrost provider architecture, `handleProviderResponse` is a utility function that only parses and returns raw response data when the `sendBackRawResponse` flag is true. It's the responsibility of each individual provider (OpenAI, Anthropic, etc.) to conditionally set `response.ExtraFields.RawResponse` using the returned raw response data based on their `sendBackRawResponse` flag. This represents a separation of concerns where the utility handles parsing and the provider handles response object construction.

Applied to files:

  • ui/lib/api.ts
📚 Learning: 2025-07-09T06:55:22.017Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T06:55:22.017Z
Learning: In the Bifrost project's ConfigStore, different functions handle sensitive values differently based on their use cases. The `restoreMetaConfigEnvVars` function preserves actual values without redaction when writing config back to file for round-trip fidelity, while `GetProviderConfigRedacted` redacts sensitive values for external API responses for security purposes. This different handling is intentional and based on the specific context where each function is used.

Applied to files:

  • transports/bifrost-http/lib/store.go
  • ui/components/config/redis-config-form.tsx
🧬 Code Graph Analysis (5)
transports/bifrost-http/handlers/redis.go (4)
transports/bifrost-http/lib/store.go (1)
  • ConfigStore (32-47)
core/schemas/logger.go (1)
  • Logger (18-35)
transports/bifrost-http/handlers/utils.go (2)
  • SendError (27-36)
  • SendJSON (16-24)
transports/bifrost-http/lib/models.go (2)
  • DBRedisConfig (134-146)
  • DBRedisConfig (155-155)
transports/bifrost-http/main.go (3)
transports/bifrost-http/handlers/redis.go (2)
  • RedisHandler (15-18)
  • NewRedisHandler (21-26)
transports/bifrost-http/lib/config.go (1)
  • ClientConfig (11-20)
plugins/redis/main.go (2)
  • RedisPluginConfig (22-48)
  • NewRedisPlugin (83-146)
ui/lib/api.ts (1)
ui/lib/types/config.ts (1)
  • RedisConfig (134-146)
transports/bifrost-http/lib/store.go (2)
transports/bifrost-http/lib/models.go (2)
  • DBRedisConfig (134-146)
  • DBRedisConfig (155-155)
transports/bifrost-http/lib/config.go (1)
  • ClientConfig (11-20)
ui/components/config/redis-config-form.tsx (6)
ui/lib/types/config.ts (1)
  • RedisConfig (134-146)
ui/lib/api.ts (1)
  • apiService (506-506)
ui/components/ui/card.tsx (2)
  • Card (50-50)
  • CardContent (50-50)
ui/components/ui/label.tsx (1)
  • Label (24-24)
ui/components/ui/input.tsx (1)
  • Input (7-22)
ui/components/ui/switch.tsx (1)
  • Switch (37-37)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
🔇 Additional comments (7)
transports/bifrost-http/lib/config.go (1)

18-18: Field addition looks good.
The new EnableCaching flag integrates cleanly with existing config; no further concerns.

ui/lib/api.ts (1)

6-6: Import looks correct

Type import for RedisConfig aligns with usage below.

ui/lib/types/config.ts (1)

129-129: CoreConfig.enable_caching addition looks good

Matches backend ClientConfig.EnableCaching and UI toggle usage.

ui/app/config/page.tsx (2)

16-17: Imports are correct

Matches usage in the new caching section.


26-26: Default include of enable_caching is fine

Keeps defaultConfig aligned with CoreConfig shape.

transports/bifrost-http/lib/models.go (1)

113-114: EnableCaching field addition looks good

Matches CoreConfig/ClientConfig and UI toggle.

transports/bifrost-http/handlers/redis.go (1)

35-43: Confirm password exposure policy in GET response.

GetRedisConfig returns plaintext password to the UI. If the admin UI is trusted-only, this is acceptable; otherwise consider redacting and using “write-only” semantics.

Would you like to redact password in GET and only accept non-empty password on PUT to update it (keeping previous value if empty)?

Comment thread transports/bifrost-http/lib/store.go Outdated
Comment thread transports/bifrost-http/lib/store.go Outdated
Comment thread ui/components/config/redis-config-form.tsx
Comment thread ui/components/config/redis-config-form.tsx
Comment thread ui/lib/api.ts Outdated
Comment thread ui/lib/types/config.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🔭 Outside diff range comments (1)
transports/bifrost-http/main.go (1)

143-146: CORS: allow the x-bf-cache-key header

Browsers will preflight requests that include custom headers. Without whitelisting X-BF-Cache-Key, the request will be blocked.

Apply this diff:

-    ctx.Response.Header.Set("Access-Control-Allow-Headers", "Content-Type, Authorization, X-Requested-With")
+    ctx.Response.Header.Set("Access-Control-Allow-Headers", "Content-Type, Authorization, X-Requested-With, X-BF-Cache-Key")

Optional: also include X-BF-VK if governance requires it.

♻️ Duplicate comments (21)
ui/lib/utils/validation.ts (2)

318-343: Bug: parseInt accepts invalid ports like "6379abc"; enforce digits-only and trim once

Port parsing should reject partial numerics and whitespace quirks. Trim once, ensure port is all digits, then range-check.

Apply this diff:

 export function isValidRedisAddress(addr: string): boolean {
-  if (!addr || !addr.trim()) {
+  if (!addr || !addr.trim()) {
     return false
   }
 
-  // Check for host:port format
-  const parts = addr.split(':')
+  const value = addr.trim()
+  // Check for host:port format
+  const parts = value.split(':')
   if (parts.length !== 2) {
     return false
   }
 
-  const [host, port] = parts
+  const [host, portStr] = parts
 
   // Host should not be empty
   if (!host.trim()) {
     return false
   }
 
-  // Port should be a number between 1 and 65535
-  const portNum = parseInt(port, 10)
-  if (isNaN(portNum) || portNum < 1 || portNum > 65535) {
+  // Port must be digits only and between 1 and 65535
+  const port = portStr.trim()
+  if (!/^\d+$/.test(port)) {
+    return false
+  }
+  const portNum = Number(port)
+  if (portNum < 1 || portNum > 65535) {
     return false
   }
 
   return true
 }

318-343: Optional: Support bracketed IPv6 and redis:// schemes

If admins may input “[::1]:6379” or “redis://host:port”, extend validation accordingly.

Example IPv6 support:

export function isValidRedisAddress(addr: string): boolean {
  if (!addr || !addr.trim()) return false
  const value = addr.trim()

  // URL scheme support (redis:// or rediss://)
  if (value.startsWith('redis://') || value.startsWith('rediss://')) {
    try {
      const u = new URL(value)
      if (!u.hostname) return false
      const port = u.port?.trim()
      if (!/^\d+$/.test(port)) return false
      const portNum = Number(port)
      return portNum >= 1 && portNum <= 65535
    } catch {
      return false
    }
  }

  // Bracketed IPv6 [::1]:6379
  if (value.startsWith('[')) {
    const rb = value.indexOf(']')
    if (rb < 0) return false
    const host = value.slice(1, rb).trim()
    if (!host) return false
    if (value.length <= rb + 2 || value[rb + 1] !== ':') return false
    const port = value.slice(rb + 2).trim()
    if (!/^\d+$/.test(port)) return false
    const portNum = Number(port)
    return portNum >= 1 && portNum <= 65535
  }

  // host:port
  const parts = value.split(':')
  if (parts.length !== 2) return false
  const [host, portStr] = parts
  if (!host.trim()) return false
  if (!/^\d+$/.test(portStr.trim())) return false
  const portNum = Number(portStr)
  return portNum >= 1 && portNum <= 65535
}
transports/go.mod (1)

20-23: Use go.work for local overrides; keep go.mod reproducible

Move local replace directives into a top-level go.work to avoid baking local paths into the module manifest.

Proposed change in transports/go.mod:

-replace github.com/maximhq/bifrost/plugins/redis => ../plugins/redis
-
-replace github.com/maximhq/bifrost/core => ../core
+// Local development overrides should live in go.work at the repo root

Example go.work at repo root:

go 1.24

use (
  ./core
  ./plugins/redis
  ./transports
)

replace github.com/maximhq/bifrost/core => ./core
replace github.com/maximhq/bifrost/plugins/redis => ./plugins/redis
transports/bifrost-http/lib/ctx.go (1)

15-15: Avoid magic string drift: use an exported RequestCacheKey from the redis plugin

Define a constant in the plugin and reference it here to prevent key mismatches.

In plugins/redis (e.g., main.go or a context.go):

// package redis
type ContextKey string

const RequestCacheKey ContextKey = "request-cache-key"

Then update this file:

- bifrostCtx = context.WithValue(bifrostCtx, redis.ContextKey("request-cache-key"), string(value))
+ bifrostCtx = context.WithValue(bifrostCtx, redis.RequestCacheKey, strings.TrimSpace(string(value)))

Also applies to: 115-118

transports/bifrost-http/handlers/config.go (1)

88-104: Toggling EnableCaching requires service restart; surface this in API response

Runtime toggle won’t apply until restart. Return a restart indicator when the flag changes.

Apply this diff:

   updatedConfig.EnforceGovernanceHeader = req.EnforceGovernanceHeader
-  updatedConfig.EnableCaching = req.EnableCaching
+  // Track change to inform clients that restart is required
+  cachingChanged := req.EnableCaching != currentConfig.EnableCaching
+  updatedConfig.EnableCaching = req.EnableCaching
@@
- SendJSON(ctx, map[string]any{
-   "status":  "success",
-   "message": "Configuration updated successfully",
- }, h.logger)
+ SendJSON(ctx, map[string]any{
+   "status":            "success",
+   "message":           "Configuration updated successfully",
+   "restart_required":  cachingChanged, // caching on/off takes effect after restart
+ }, h.logger)

Optional follow-up: show a banner/toast in the UI when restart_required is true so admins aren’t surprised.

ui/lib/api.ts (2)

243-250: Fix updateRedisConfig() response typing to match backend payload

PUT /config/redis returns only status/message (no config). The current signature expects { config: RedisConfig }, causing downstream type mismatches.

Apply this diff:

-  async updateRedisConfig(data: RedisConfig): ServiceResponse<{ config: RedisConfig }> {
+  async updateRedisConfig(data: RedisConfig): ServiceResponse<{ status: string; message: string }> {
     try {
       const response = await this.client.put('/config/redis', data)
       return [response.data, null]
     } catch (error) {
       return [null, this.getErrorMessage(error)]
     }
   }

234-241: Align GET vs PUT response shapes (avoid special-casing in consumers)

GET returns a bare RedisConfig, while PUT returns a status/message envelope (after the fix above). Pick a consistent shape across methods or clearly document the difference to avoid ad-hoc handling in the UI.

ui/app/config/page.tsx (1)

337-346: Let users configure Redis immediately after toggling caching

Rendering RedisConfigForm only when both configInDB.enable_caching and config.enable_caching are true blocks first-time configuration until after restart.

Apply this diff to render based on the current toggle:

-              {configInDB.enable_caching && config.enable_caching && (
+              {config.enable_caching && (
                 <div className="mt-4 space-y-4">
                   <Separator />
                   <RedisConfigForm />
                 </div>
               )}

The restart banner below already signals when a restart is needed.

transports/bifrost-http/main.go (2)

436-436: Avoid hard-coding the cache context key

Using a literal "request-cache-key" couples main.go with any context extraction logic elsewhere. Prefer a shared constant (e.g., in transports/bifrost-http/lib) used both by the plugin config and the header-extraction code to prevent drift.


421-451: Don’t crash on first-time enable; register Redis config routes regardless of plugin state

Current flow fatals if the config row is missing and only registers the Redis routes when caching is enabled and plugin init succeeds. This prevents the UI from creating the initial config.

Proposed changes:

  • Always register Redis config routes (so the UI can create/update the row).
  • Treat gorm.ErrRecordNotFound as a soft condition: skip plugin init, log a warning, keep the handler available.
  • Keep fatal only for unexpected errors.

Diff (imports and Redis block):

@@
-import (
+import (
+   "errors"
@@
 )
@@
-  var redisHandler *handlers.RedisHandler
+  // Register Redis config handler unconditionally so the UI can manage config
+  redisHandler := handlers.NewRedisHandler(store, logger)
 
-  if store.ClientConfig.EnableCaching {
+  if store.ClientConfig.EnableCaching {
     // Get Redis configuration from database
     redisDBConfig, err := store.GetRedisConfig()
-    if err != nil {
-      log.Fatalf("failed to get Redis config: %v", err)
-    }
+    if err != nil {
+      if errors.Is(err, gorm.ErrRecordNotFound) {
+        log.Println("warning: Redis config not found; skipping Redis plugin initialization (UI can create it via /config/redis)")
+      } else {
+        log.Fatalf("failed to get Redis config: %v", err)
+      }
+    } else {
 
-    // Convert DBRedisConfig to RedisPluginConfig
-    pluginConfig := redis.RedisPluginConfig{
+      // Convert DBRedisConfig to RedisPluginConfig
+      pluginConfig := redis.RedisPluginConfig{
         Addr:            redisDBConfig.Addr,
         Username:        redisDBConfig.Username,
         Password:        redisDBConfig.Password,
         DB:              redisDBConfig.DB,
         CacheKey:        "request-cache-key", // Always use this key as specified
         TTL:             time.Duration(redisDBConfig.TTLSeconds) * time.Second,
         Prefix:          redisDBConfig.Prefix,
         CacheByModel:    &redisDBConfig.CacheByModel,
         CacheByProvider: &redisDBConfig.CacheByProvider,
-    }
+      }
 
-    redisPlugin, err := redis.NewRedisPlugin(pluginConfig, logger)
-    if err != nil {
-      log.Fatalf("failed to initialize Redis plugin: %v", err)
-    }
+      redisPlugin, err := redis.NewRedisPlugin(pluginConfig, logger)
+      if err != nil {
+        log.Fatalf("failed to initialize Redis plugin: %v", err)
+      }
 
-    loadedPlugins = append(loadedPlugins, redisPlugin)
-
-    redisHandler = handlers.NewRedisHandler(store, logger)
+      loadedPlugins = append(loadedPlugins, redisPlugin)
+    }
   }
@@
-  if redisHandler != nil {
-    redisHandler.RegisterRoutes(r)
-  }
+  // Redis config endpoints should always be available
+  redisHandler.RegisterRoutes(r)

This keeps the system operable on first enablement and lets the UI seed the config row.

Also applies to: 501-503, 54-85

transports/bifrost-http/lib/models.go (2)

142-143: Set defaults for CacheByModel/Provider to preserve expected behavior

Passing non-nil pointers disables plugin-side defaults. Without DB defaults, both flags default to false and reduce cache hit rates unintentionally.

Apply:

-  CacheByModel    bool      `gorm:"" json:"cache_by_model"`                      // Include model in cache key
-  CacheByProvider bool      `gorm:"" json:"cache_by_provider"`                   // Include provider in cache key
+  CacheByModel    bool      `gorm:"default:true" json:"cache_by_model"`          // Include model in cache key
+  CacheByProvider bool      `gorm:"default:true" json:"cache_by_provider"`       // Include provider in cache key

Note: Backfill existing rows if necessary.


133-147: Add validation hook to prevent invalid Redis rows from crashing plugin init

Validate minimal invariants before save: non-empty Addr in host:port form and TTLSeconds > 0. Fail early rather than at plugin startup.

Suggested addition:

@@
 type DBRedisConfig struct {
@@
 }
 
+// Basic validation for Redis configuration
+func (r *DBRedisConfig) BeforeSave(tx *gorm.DB) error {
+  if strings.TrimSpace(r.Addr) == "" {
+    return fmt.Errorf("redis addr is required (host:port)")
+  }
+  if r.TTLSeconds <= 0 {
+    return fmt.Errorf("ttl_seconds must be > 0")
+  }
+  return nil
+}

You’ll need:

-import (
+import (
   "encoding/json"
   "time"
+  "fmt"
+  "strings"
transports/bifrost-http/lib/store.go (4)

65-66: Changelog/doc update for new flag.

EnableCaching added to defaults. Please add a changelog entry and update sample config/docs so downstream users discover it.


238-249: Avoid unconditional AutoMigrate on Redis table at startup.

Including &DBRedisConfig{} in AutoMigrate causes a schema diff on every boot. Wrap it with a HasTable/needs-upgrade guard to reduce lock contention.


871-889: Single-row assumption for client config.

First(&dbConfig) will return an arbitrary row if duplicates exist. Enforce uniqueness or use Take with explicit ordering to be deterministic.


2104-2111: Wrap delete+create in a transaction for atomic Redis config updates.

Current flow can leave the table empty if Create fails after Delete. Use a transaction.

-func (s *ConfigStore) UpdateRedisConfig(config *DBRedisConfig) error {
-  // Delete existing Redis config and create new one
-  if err := s.db.Session(&gorm.Session{AllowGlobalUpdate: true}).Delete(&DBRedisConfig{}).Error; err != nil {
-    return err
-  }
-  return s.db.Create(config).Error
-}
+func (s *ConfigStore) UpdateRedisConfig(config *DBRedisConfig) error {
+  return s.db.Transaction(func(tx *gorm.DB) error {
+    if err := tx.Session(&gorm.Session{AllowGlobalUpdate: true}).Delete(&DBRedisConfig{}).Error; err != nil {
+      return err
+    }
+    return tx.Create(config).Error
+  })
+}
transports/bifrost-http/handlers/redis.go (2)

54-64: Add server-side validation for Addr format, DB range, and TTL sanity.

Currently only checks non-empty Addr and coerces TTL <= 0 to 300. Add:

  • Addr shape sanity (must contain ":" and non-empty host).
  • DB in a reasonable range (e.g., 0–15 for common Redis deployments).
  • Reject TTL < 1 instead of silently coercing.
 // Validate required fields
 if req.Addr == "" {
   SendError(ctx, fasthttp.StatusBadRequest, "Redis address is required", h.logger)
   return
 }
+// Minimal addr sanity: "<host>:<port>"
+if host, port, ok := strings.Cut(req.Addr, ":"); !ok || host == "" || port == "" {
+  SendError(ctx, fasthttp.StatusBadRequest, "Redis address must be in host:port format", h.logger)
+  return
+}
 
-// Validate TTL
-if req.TTLSeconds <= 0 {
-  req.TTLSeconds = 300 // Default to 5 minutes
-}
+// Validate DB number (common default range)
+if req.DB < 0 || req.DB > 15 {
+  SendError(ctx, fasthttp.StatusBadRequest, "Redis database number must be between 0 and 15", h.logger)
+  return
+}
+// Validate TTL
+if req.TTLSeconds < 1 {
+  SendError(ctx, fasthttp.StatusBadRequest, "TTL must be >= 1 second", h.logger)
+  return
+}

Note: add strings to imports.


73-79: Drop redundant status setting before SendJSON.

SendJSON already sets status and headers. Remove ctx.SetStatusCode to avoid double-setting.

-ctx.SetStatusCode(fasthttp.StatusOK)
 SendJSON(ctx, map[string]any{
   "status":  "success",
   "message": "Redis configuration updated successfully",
   "config":  req,
 }, h.logger)
ui/components/config/redis-config-form.tsx (3)

28-33: Use portable timeout ref type (works in browsers and edge runtimes).

NodeJS.Timeout requires the “node” lib. Use ReturnType.

-const addrTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-const usernameTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-const passwordTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-const prefixTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-const ttlTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
+const addrTimeoutRef = useRef<ReturnType<typeof setTimeout>>()
+const usernameTimeoutRef = useRef<ReturnType<typeof setTimeout>>()
+const passwordTimeoutRef = useRef<ReturnType<typeof setTimeout>>()
+const prefixTimeoutRef = useRef<ReturnType<typeof setTimeout>>()
+const ttlTimeoutRef = useRef<ReturnType<typeof setTimeout>>()

63-85: Fix state reversion and avoid clobbering concurrent edits.

updateConfig closes over stale config and reverts with setConfig(config). Store the previous before updating and use functional updates.

-const updateConfig = async (updates: Partial<RedisConfig>) => {
-  const newConfig = { ...config, ...updates }
-  setConfig(newConfig)
+const updateConfig = async (updates: Partial<RedisConfig>) => {
+  const previousConfig = config
+  setConfig(prev => ({ ...prev, ...updates }))
   try {
-    const [_, error] = await apiService.updateRedisConfig(newConfig)
+    const [_, error] = await apiService.updateRedisConfig({ ...previousConfig, ...updates })
     if (error) {
       if (error.includes('status code 405')) {
         toast.error('Please enable redis plugin and restart Bifrost.')
         return
       }
       toast.error(error)
-      setConfig(config)
+      setConfig(previousConfig)
     } else {
       toast.success('Redis configuration updated successfully')
     }
   } catch (error) {
     toast.error('Failed to update Redis configuration')
-    setConfig(config)
+    setConfig(previousConfig)
   }
 }

88-156: Reduce debounce duplication with a reusable helper.

Five handlers differ only by field key. Extract a generic debounced field updater to cut ~50 lines without hurting readability.

// Example
const createDebouncedHandler = <K extends keyof RedisConfig>(
  field: K,
  ref: React.MutableRefObject<ReturnType<typeof setTimeout> | undefined>
) => (value: RedisConfig[K]) => {
  setConfig(prev => ({ ...prev, [field]: value }))
  if (ref.current) clearTimeout(ref.current)
  ref.current = setTimeout(() => updateConfig({ [field]: value } as Partial<RedisConfig>), 1000)
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f7ce6c and 8431c85.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (13)
  • transports/bifrost-http/handlers/config.go (1 hunks)
  • transports/bifrost-http/handlers/redis.go (1 hunks)
  • transports/bifrost-http/lib/config.go (1 hunks)
  • transports/bifrost-http/lib/ctx.go (2 hunks)
  • transports/bifrost-http/lib/models.go (2 hunks)
  • transports/bifrost-http/lib/store.go (6 hunks)
  • transports/bifrost-http/main.go (3 hunks)
  • transports/go.mod (3 hunks)
  • ui/app/config/page.tsx (3 hunks)
  • ui/components/config/redis-config-form.tsx (1 hunks)
  • ui/lib/api.ts (2 hunks)
  • ui/lib/types/config.ts (1 hunks)
  • ui/lib/utils/validation.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (55)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T04:58:08.229Z
Learning: Pratham-Mishra04 prefers not to add logging or error handling for unreachable code paths in the Bifrost project. When provider types or similar entities are predefined in the system, defensive programming like logging in default cases is considered unnecessary overhead.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: docs/usage/go-package/mcp.md:408-412
Timestamp: 2025-07-01T12:40:08.576Z
Learning: Pratham-Mishra04 is okay with keeping bullet list formatting that uses colons after dashes in markdown documentation, even if it triggers linter warnings, preferring functionality over strict formatting rules.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: tests/core-providers/scenarios/chat_completion_stream.go:103-105
Timestamp: 2025-07-16T04:26:09.288Z
Learning: Pratham-Mishra04 prefers to keep test code simple when it serves its basic functional purpose. For tests that are meant to validate core functionality (like verifying streaming works), they consider hard-coded reasonable limits acceptable rather than making them configurable.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:880-910
Timestamp: 2025-07-08T17:14:21.544Z
Learning: Pratham-Mishra04 prefers resilient system design where missing environment variables for MCP connections should not cause complete system failure. The system should continue processing other MCP connections even when some fail, maintaining partial functionality rather than implementing fail-fast behavior.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: transports/README.md:26-28
Timestamp: 2025-07-01T12:45:06.906Z
Learning: Pratham-Mishra04 prefers keeping documentation examples simple and concise, trusting users to handle production-specific considerations like version pinning themselves rather than cluttering examples with additional notes.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:1081-1098
Timestamp: 2025-07-08T17:16:50.811Z
Learning: Pratham-Mishra04 prefers practical redaction approaches over theoretical security improvements when the threat model is low-risk, such as admin-only interfaces in the Bifrost project. Fixed-length redaction is acceptable when only trusted administrators will see the redacted values.
📚 Learning: 2025-08-07T13:33:18.094Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:0-0
Timestamp: 2025-08-07T13:33:18.094Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), the context key for enabling per-request JSON parsing is "enable-streaming-json-parser" (defined as EnableStreamingJSONParser constant), which is distinct from the plugin name constant to avoid confusion between plugin identification and per-request activation control.

Applied to files:

  • transports/bifrost-http/lib/config.go
📚 Learning: 2025-08-03T20:36:21.906Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:183-191
Timestamp: 2025-08-03T20:36:21.906Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers the current string matching approach for determining rate limit violation decision types (DecisionTokenLimited vs DecisionRequestLimited) adequate and not important enough to refactor with more robust explicit checks, preferring functional simplicity over theoretical robustness improvements.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
  • transports/go.mod
📚 Learning: 2025-08-03T20:50:48.247Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/tracker.go:146-146
Timestamp: 2025-08-03T20:50:48.247Z
Learning: In the Bifrost governance tracker (transports/bifrost-http/plugins/governance/tracker.go), Pratham-Mishra04 implements database persistence for reset operations using arrays to collect reset entities (resetRateLimits and resetBudgets) and then batch persist them using t.db.Save() with proper error aggregation in the PerformStartupResets method.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called in two specific scenarios: when ErrorDetails is present (error case) and when isFinalChunk is true (end of streaming). It is not called repeatedly during streaming, making performance optimization with caching unnecessary.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called at final chunk of streaming responses and when errors are returned, not repeatedly during streaming. This makes the performance impact of the database query minimal and doesn't warrant optimization with in-memory caching.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and always returns a pointer to a valid context. It starts with context.Background() and only adds values to it, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-17T14:04:21.104Z
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
  • transports/go.mod
📚 Learning: 2025-08-03T20:11:26.945Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:220-220
Timestamp: 2025-08-03T20:11:26.945Z
Learning: In the Bifrost HTTP transport handlers using fasthttp router, path parameters extracted via ctx.UserValue() can be safely type-asserted to string without additional checks because the router guarantees their presence and type when routes match. Pratham-Mishra04 confirmed that defensive type assertion checks are unnecessary overhead in this context.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-04T22:19:12.975Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#212
File: transports/bifrost-http/integrations/openai/types.go:250-288
Timestamp: 2025-08-04T22:19:12.975Z
Learning: In transports/bifrost-http/integrations/openai/types.go, the ConvertToBifrostRequest function for OpenAIEmbeddingRequest handles []interface{} input without validation for non-string elements because Pratham-Mishra04 confirmed this case will never happen in practice, consistent with their preference to avoid defensive programming for unreachable scenarios.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-15T14:24:49.882Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/handlers/redis.go
  • transports/go.mod
📚 Learning: 2025-08-08T14:12:14.754Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#238
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-08-08T14:12:14.754Z
Learning: In transports/bifrost-http/plugins/logging/main.go, stream accumulator cleanup is intended to use a 5-minute TTL (consistent with processing logs). Comments should state “older than 5 minutes,” and using central constants (processingLogTTL, streamAccumulatorTTL) helps avoid future mismatches.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-10T13:51:52.859Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:140-146
Timestamp: 2025-06-10T13:51:52.859Z
Learning: In Bifrost core v1.0.9, ImageContent.Type was a pointer type (*string accessed via bifrost.Ptr), but in v1.0.10 it was changed to a value type (ImageContentType). When reviewing code, check the core version being used to determine the correct assignment pattern.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-04T09:22:18.123Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/bifrost.go:46-49
Timestamp: 2025-06-04T09:22:18.123Z
Learning: In core/schemas/bifrost.go, the RequestInput struct uses ChatCompletionInput *[]BifrostMessage (pointer-to-slice) rather than []BifrostMessage to properly represent union type semantics. For text completion requests, ChatCompletionInput should be nil to indicate "no chat payload at all", while for chat completion requests it should be non-nil (even if empty slice). This distinguishes between different request types rather than just empty vs non-empty chat messages.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-26T07:37:24.385Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#131
File: transports/bifrost-http/lib/config.go:35-68
Timestamp: 2025-06-26T07:37:24.385Z
Learning: In the Bifrost project's MCP configuration handling, empty environment variables should be treated as missing/invalid rather than as valid empty values. The current implementation using `os.Getenv(envKey); envValue != ""` to check for non-empty values is the desired behavior.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-08-08T15:31:28.737Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#238
File: transports/bifrost-http/plugins/logging/main.go:687-703
Timestamp: 2025-08-08T15:31:28.737Z
Learning: In transports/bifrost-http/plugins/logging/main.go, TejasGhatte prefers keeping separate helpers for streaming detection: isStreamingResponse (any streaming) and isTextStreamingResponse (text-only). Avoid parameterizing/merging them, as that adds code and reduces clarity.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-05T20:43:59.593Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#214
File: core/providers/azure.go:49-50
Timestamp: 2025-08-05T20:43:59.593Z
Learning: In core/providers/azure.go, the azureTextCompletionResponsePool should use AzureTextResponse type, not schemas.BifrostResponse, to maintain consistency with the acquireAzureTextResponse() and releaseAzureTextResponse() functions that work with *AzureTextResponse objects.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-07-10T13:44:14.518Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.518Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-10T13:44:39.237Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:16-18
Timestamp: 2025-07-10T13:44:39.237Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers to ignore JSON marshaling errors when storing log entries because logging is not critical for their use case and they are certain the marshaling operations won't fail.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-08T18:09:32.147Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-07-08T18:09:32.147Z
Learning: In the Bifrost HTTP transport logging plugin (transports/bifrost-http/plugins/logging/main.go), the DatabasePath is explicitly set from main.go via a config struct pointing to appDir/logs, so the default value in the NewLoggerPlugin constructor is only used for standalone plugin usage and doesn't affect the main application flow.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
  • transports/go.mod
📚 Learning: 2025-06-27T17:07:39.462Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#135
File: docs/core-package.md:105-116
Timestamp: 2025-06-27T17:07:39.462Z
Learning: In Go, when importing a package, the identifier used to access it is determined by the `package` declaration in the Go source files, not the directory name. For the Bifrost project, the core directory files declare `package bifrost`, so importing `"github.com/maximhq/bifrost/core"` automatically makes it accessible as `bifrost.Init`, `bifrost.Ptr`, etc., without needing an explicit alias.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/go.mod
📚 Learning: 2025-07-10T13:44:23.297Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:94-111
Timestamp: 2025-07-10T13:44:23.297Z
Learning: Pratham-Mishra04 prefers not to add error handling for JSON marshaling operations in the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go) because logging is not critical functionality and the structured schema data being marshaled is unlikely to fail. They accept the risk of not handling json.Marshal errors in logging contexts to keep the code simple.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-08T06:51:27.123Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/schemas/bifrost.go:686-692
Timestamp: 2025-08-08T06:51:27.123Z
Learning: In core/schemas/bifrost.go, BifrostError.StreamControl is intended for plugin developers only and is excluded from JSON serialization; its presence controls streaming behavior internally.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-16T04:29:53.409Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use `package main` for test files in the tests/core-providers directory rather than more descriptive package names like `coreproviders_test`.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/go.mod
📚 Learning: 2025-06-16T04:13:55.437Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/go.mod
📚 Learning: 2025-07-18T10:52:24.942Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#159
File: docs/usage/http-transport/README.md:0-0
Timestamp: 2025-07-18T10:52:24.942Z
Learning: In the Bifrost project, the npx wrapper (`npx -y bifrostlatest -port 8080`) correctly handles single-dash long flags like `-port` without requiring double dashes or the `--` separator. The implementation properly forwards flags to the underlying Go binary.

Applied to files:

  • ui/lib/utils/validation.ts
📚 Learning: 2025-08-08T07:07:08.801Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/providers/openai.go:459-459
Timestamp: 2025-08-08T07:07:08.801Z
Learning: In core/providers/utils.go, processAndSendResponse and processAndSendError now take exactly five parameters: (ctx, postHookRunner, response|err, responseChan, logger). All providers (OpenAI, Anthropic, Cohere, Bedrock) must call these with the logger as the fifth argument; a repo-wide check in PR maximhq/bifrost#219 confirmed this is already consistent.

Applied to files:

  • ui/lib/api.ts
  • transports/bifrost-http/handlers/redis.go
  • transports/go.mod
📚 Learning: 2025-07-29T16:10:52.088Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#196
File: core/providers/openai.go:180-183
Timestamp: 2025-07-29T16:10:52.088Z
Learning: In the Bifrost provider architecture, `handleProviderResponse` is a utility function that only parses and returns raw response data when the `sendBackRawResponse` flag is true. It's the responsibility of each individual provider (OpenAI, Anthropic, etc.) to conditionally set `response.ExtraFields.RawResponse` using the returned raw response data based on their `sendBackRawResponse` flag. This represents a separation of concerns where the utility handles parsing and the provider handles response object construction.

Applied to files:

  • ui/lib/api.ts
📚 Learning: 2025-06-09T14:03:34.227Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:0-0
Timestamp: 2025-06-09T14:03:34.227Z
Learning: In the Bifrost HTTP transport layer (transports/bifrost-http/integrations/), request validation like checking for empty messages should be handled by the provider rather than at the transport layer. The transport layer should forward requests to Bifrost core/providers for validation.

Applied to files:

  • ui/lib/api.ts
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-03T20:53:08.832Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: ui/app/governance/page.tsx:29-60
Timestamp: 2025-08-03T20:53:08.832Z
Learning: In the Bifrost governance page (ui/app/governance/page.tsx), Pratham-Mishra04 intentionally keeps the loading state active when critical errors occur (such as governance being disabled or core config failures) to prevent showing a broken or incomplete governance interface to users. This is a deliberate UX design choice to avoid presenting non-functional page states.

Applied to files:

  • ui/app/config/page.tsx
  • ui/components/config/redis-config-form.tsx
📚 Learning: 2025-06-20T16:21:18.912Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/go.mod
📚 Learning: 2025-07-08T18:20:24.086Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-07-08T17:31:44.662Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#145
File: transports/bifrost-http/main.go:124-128
Timestamp: 2025-07-08T17:31:44.662Z
Learning: Pratham-Mishra04 prefers to keep the CORS middleware simple in the Bifrost HTTP transport (transports/bifrost-http/main.go) rather than adding port validation for localhost origins, considering the current implementation sufficient for the intended use case.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-09T16:35:26.914Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#64
File: transports/bifrost-http/integrations/genai/types.go:273-313
Timestamp: 2025-06-09T16:35:26.914Z
Learning: In convertGenerationConfigToParams method in transports/bifrost-http/integrations/genai/types.go, pre-allocating the ExtraParams map is preferred over lazy allocation because the method has multiple potential ExtraParams assignments, making the computational overhead of conditional checks exceed the memory savings of an empty map.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-06-15T16:07:53.140Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#85
File: core/providers/anthropic.go:150-156
Timestamp: 2025-06-15T16:07:53.140Z
Learning: In the Bifrost codebase, constructor functions are allowed to mutate input ProviderConfig objects in-place (e.g., setting default BaseURL values and trimming trailing slashes). This pattern is acceptable and doesn't need to be flagged as a code review issue.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
  • ui/components/config/redis-config-form.tsx
📚 Learning: 2025-08-02T13:02:13.642Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#174
File: transports/bifrost-http/lib/models.go:251-254
Timestamp: 2025-08-02T13:02:13.642Z
Learning: In the Bifrost project's database models (transports/bifrost-http/lib/models.go), Pratham-Mishra04 considers meta config types to be predefined and expects that unknown meta config types will never occur in practice. The current implementation that returns nil for unknown types in the AfterFind hook is intentional, as defensive programming for this scenario is considered unnecessary.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-06-16T14:44:42.893Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:44:42.893Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to return a validation error rather than setting a default role value. This represents a design preference for strict input validation over silent error correction.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-03T20:28:00.857Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:1038-1049
Timestamp: 2025-08-03T20:28:00.857Z
Learning: In the Bifrost governance handler (transports/bifrost-http/handlers/governance.go), Pratham-Mishra04 is comfortable with exposing full error details including err.Error() in production responses for governance API endpoints, prioritizing simplicity over potential information disclosure concerns.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-15T14:34:29.401Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: core/providers/ollama.go:59-61
Timestamp: 2025-06-15T14:34:29.401Z
Learning: The `CheckAndSetDefaults()` method on `*schemas.ProviderConfig` in Go does not return any error - it has a void return type and only sets default values on the configuration struct.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-06-10T11:19:29.604Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.

Applied to files:

  • transports/bifrost-http/main.go
📚 Learning: 2025-06-10T11:19:29.604Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-09T06:55:22.017Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T06:55:22.017Z
Learning: In the Bifrost project's ConfigStore, different functions handle sensitive values differently based on their use cases. The `restoreMetaConfigEnvVars` function preserves actual values without redaction when writing config back to file for round-trip fidelity, while `GetProviderConfigRedacted` redacts sensitive values for external API responses for security purposes. This different handling is intentional and based on the specific context where each function is used.

Applied to files:

  • transports/bifrost-http/lib/store.go
  • ui/components/config/redis-config-form.tsx
📚 Learning: 2025-08-03T20:17:11.876Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:68-74
Timestamp: 2025-08-03T20:17:11.876Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers nil validation in constructors like NewBudgetResolver unnecessary because the parameters will never be nil in practice. This aligns with their general preference to avoid defensive programming for scenarios they consider unreachable.

Applied to files:

  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
  • transports/go.mod
📚 Learning: 2025-07-08T16:40:59.098Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:286-292
Timestamp: 2025-07-08T16:40:59.098Z
Learning: Pratham-Mishra04 prefers to keep simpler error handling patterns when errors are unlikely to occur in practice, such as safety checks in BadgerDB iterations where item.Value() is called on valid items. The user considers the overhead of explicit error handling not worth it in such scenarios.

Applied to files:

  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-16T14:50:46.859Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:22-56
Timestamp: 2025-06-16T14:50:46.859Z
Learning: In the Google GenAI integration at transports/bifrost-http/integrations/genai/types.go, the manual URL-safe base64 decoding implementation (converting - to +, _ to /, and adding padding) is required because base64.RawURLEncoding.DecodeString fails for the specific url encoded bytes format being handled.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-16T04:13:42.755Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-08T16:48:25.386Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:509-514
Timestamp: 2025-07-08T16:48:25.386Z
Learning: In the Bifrost logging system (transports/bifrost-http/plugins/logging/utils.go), the Content field in message structures is a struct value type, not a pointer, so it will never be nil and doesn't require nil checks. However, ContentStr within Content is a pointer and should be checked for nil.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-04T11:27:45.020Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#208
File: transports/bifrost-http/handlers/completions.go:221-225
Timestamp: 2025-08-04T11:27:45.020Z
Learning: In transports/bifrost-http/handlers/completions.go, Pratham-Mishra04 intentionally delegates file size validation for audio transcription uploads to the downstream API/provider rather than enforcing MaxFileSize limits at the HTTP transport layer. This follows their architectural preference of keeping the transport layer simple and letting providers handle domain-specific validation concerns, even for potential memory exhaustion scenarios.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-16T03:55:16.949Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T03:55:16.949Z
Learning: Go 1.24 was released in February 2025 and is stable and available for use in go.mod files.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-04T03:57:50.981Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-07-08T18:12:13.590Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: docs/contributing/README.md:22-27
Timestamp: 2025-07-08T18:12:13.590Z
Learning: In the Bifrost project, the tests directory structure has `tests/core-providers/` and `tests/transports-integrations/` as sibling directories. From `tests/core-providers/`, the correct relative path to reach `tests/transports-integrations/` is `../transports-integrations/`, not `../../tests/transports-integrations/`.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-16T03:54:48.005Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:38-38
Timestamp: 2025-06-16T03:54:48.005Z
Learning: The `core-providers-test` module in `tests/core-providers/` is an internal testing module that will never be consumed as a dependency by external projects, so the replace directive pointing to `../../core` is acceptable for local development and testing purposes.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-19T16:57:25.177Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:30-36
Timestamp: 2025-06-19T16:57:25.177Z
Learning: In the bifrost repository, Pratham-Mishra04 prefers to keep GitHub Actions workflows lean and trusts their controlled tagging process for core releases, avoiding unnecessary validation steps that they consider overkill.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-16T04:12:05.427Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.

Applied to files:

  • transports/go.mod
🧬 Code Graph Analysis (4)
ui/lib/api.ts (1)
ui/lib/types/config.ts (1)
  • RedisConfig (134-146)
transports/bifrost-http/main.go (3)
transports/bifrost-http/handlers/redis.go (2)
  • RedisHandler (15-18)
  • NewRedisHandler (21-26)
transports/bifrost-http/lib/config.go (1)
  • ClientConfig (11-20)
plugins/redis/main.go (2)
  • RedisPluginConfig (22-48)
  • NewRedisPlugin (83-146)
transports/bifrost-http/lib/store.go (2)
transports/bifrost-http/lib/models.go (2)
  • DBRedisConfig (134-146)
  • DBRedisConfig (155-155)
transports/bifrost-http/lib/config.go (1)
  • ClientConfig (11-20)
transports/bifrost-http/handlers/redis.go (4)
transports/bifrost-http/lib/store.go (1)
  • ConfigStore (32-47)
core/schemas/logger.go (1)
  • Logger (18-35)
transports/bifrost-http/handlers/utils.go (2)
  • SendError (27-36)
  • SendJSON (16-24)
transports/bifrost-http/lib/models.go (2)
  • DBRedisConfig (134-146)
  • DBRedisConfig (155-155)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
🔇 Additional comments (9)
transports/bifrost-http/lib/config.go (1)

18-18: EnableCaching toggle added consistently to ClientConfig

Field name and JSON tag match existing patterns. No issues from this change alone.

transports/go.mod (1)

12-12: Prep for release: replace pseudo-version with a real tag when removing local replace

Current dependency is a placeholder (v0.0.0-00010101000000-000000000000). Fine for local dev with replace, but ensure a proper version/tag is used prior to release builds.

If you want, I can help bump this to the latest tagged version once the plugin is tagged.

ui/lib/types/config.ts (2)

129-131: Add enable_caching flag — LGTM

Type addition is consistent with backend and UI usage.


133-147: Clarify and Harden Redis Password Update Semantics

We need to ensure we never accidentally expose or erase Redis credentials when the UI reads or updates config:

  • Backend contracts (please verify):

    • GET /config/redis should never return the raw password. It must return a fixed-length masked sentinel (e.g. "••••••••") or omit the field entirely.
    • PUT /config/redis semantics:
      • No password field in the payload → preserve the existing password.
      • password: "" → explicitly clear the stored password.
      • password: "<new>" → update to the new value.
  • UI changes:

    • Keep the password?: string type (optional).
    • In the HTTP payload, include password only when the user has actively changed it:
      // before sending:
      const body: Partial<RedisConfig> = { …otherFields }
      if (passwordInputChanged) {
        body.password = newPassword  // empty string to clear, non-empty to update
      }
    • Don’t bind the masked sentinel back into the form value (otherwise it’ll be sent as “changed”).

Would you like me to prepare a follow-up PR to:

  1. Update the UI to only send password when modified?
  2. Coordinate with the backend to implement redaction + no-change/clear semantics?
transports/bifrost-http/lib/models.go (2)

113-120: EnableCaching on DBClientConfig — LGTM

The field integrates cleanly with existing config serialization.


155-156: Table name for config_redis — LGTM

Explicit table naming matches the existing naming convention.

transports/bifrost-http/lib/store.go (2)

311-320: EnableCaching mapping looks correct.

DB → in-memory mapping includes EnableCaching; consistent with defaults and JSON tags.


691-702: Persisting EnableCaching looks correct.

Saving EnableCaching in DBClientConfig is wired properly.

ui/components/config/redis-config-form.tsx (1)

219-225: TTL input handling is reasonable.

parseInt(...) || 300 avoids NaN rendering and noisy updates; pairs well with server-side validation.

Comment thread transports/bifrost-http/handlers/redis.go Outdated
Comment thread transports/bifrost-http/lib/store.go Outdated
Comment thread ui/components/config/redis-config-form.tsx
Comment thread ui/components/config/redis-config-form.tsx
Comment thread ui/components/config/redis-config-form.tsx
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🔭 Outside diff range comments (1)
transports/bifrost-http/main.go (1)

143-145: Include custom CORS headers for X-BF-VK and X-BF-CACHE-KEY

We’ve confirmed both x-bf-vk and x-bf-cache-key are used throughout the HTTP transport (governance plugin, ctx.go, UI, tests), so they must be explicitly allowed in preflight responses.

• File: transports/bifrost-http/main.go
Lines 143–145

-   ctx.Response.Header.Set("Access-Control-Allow-Headers", "Content-Type, Authorization, X-Requested-With")
+   // Allow requested headers plus our governance and caching headers
+   requested := string(ctx.Request.Header.Peek("Access-Control-Request-Headers"))
+   base := "Content-Type, Authorization, X-Requested-With, X-BF-VK, X-BF-CACHE-KEY"
+   if requested != "" {
+       base += ", " + requested
+   }
+   ctx.Response.Header.Set("Access-Control-Allow-Headers", base)

Please verify in your browser’s console that the Access-Control-Request-Headers sent by the client are correctly reflected in Access-Control-Allow-Headers and adjust as needed.

♻️ Duplicate comments (22)
ui/lib/utils/validation.ts (2)

319-341: Fix port validation: parseInt accepts partial numerics; trim once and require digits-only

parseInt("6379abc", 10) passes. Also trim once before splitting to avoid whitespace pitfalls.

Apply this diff:

 export function isValidRedisAddress(addr: string): boolean {
-  if (!addr || !addr.trim()) {
+  if (!addr) {
+    return false
+  }
+  const value = addr.trim()
+  if (!value) {
     return false
   }
 
-  // Check for host:port format
-  const parts = addr.split(':')
+  // Check for host:port format
+  const parts = value.split(':')
   if (parts.length !== 2) {
     return false
   }
 
-  const [host, port] = parts
+  const [host, portStr] = parts
 
   // Host should not be empty
   if (!host.trim()) {
     return false
   }
 
-  // Port should be a number between 1 and 65535
-  const portNum = parseInt(port, 10)
-  if (isNaN(portNum) || portNum < 1 || portNum > 65535) {
+  // Port must be digits-only and between 1 and 65535
+  const port = portStr.trim()
+  if (!/^\d+$/.test(port)) {
+    return false
+  }
+  const portNum = Number(port)
+  if (portNum < 1 || portNum > 65535) {
     return false
   }
 
   return true
 }

313-343: Optional: Support bracketed IPv6 and redis:// URLs

Current split(':') rejects valid forms like "[::1]:6379" and common schemes like redis://host:6379.

If in scope, extend to handle:

  • Bracketed IPv6: [ipv6]:port
  • redis:// and rediss:// URLs (use URL parsing to extract hostname/port)
    I can provide a drop-in version that covers these cases if you want.
transports/bifrost-http/lib/ctx.go (1)

15-15: Avoid magic string for cache context key; use a plugin-exported const

Use a shared constant from the redis plugin instead of a typed string literal to prevent drift.

Apply this diff here:

-            bifrostCtx = context.WithValue(bifrostCtx, redis.ContextKey("request-cache-key"), string(value))
+            bifrostCtx = context.WithValue(bifrostCtx, redis.RequestCacheKey, string(value))

And in plugins/redis (outside this file), define the const:

// package redis
type ContextKey string

const RequestCacheKey ContextKey = "request-cache-key"

Also applies to: 115-118

transports/bifrost-http/handlers/config.go (1)

88-88: Toggling caching isn’t hot-reloaded; surface restart_required in response

EnableCaching is persisted but not applied until restart (plugin is initialized only at startup). Return an indicator so users aren’t surprised.

Minimal change:

   // Get current config with proper locking
   currentConfig := h.store.ClientConfig
   updatedConfig := currentConfig
+  cachingChanged := req.EnableCaching != currentConfig.EnableCaching

   ...
   updatedConfig.EnableCaching = req.EnableCaching

   // Update the store with the new config
   h.store.ClientConfig = updatedConfig
   ...
   ctx.SetStatusCode(fasthttp.StatusOK)
   SendJSON(ctx, map[string]any{
     "status":  "success",
     "message": "Configuration updated successfully",
+    "restart_required": cachingChanged,
   }, h.logger)

Optionally, mirror this in the UI with a banner when the toggle changes.

transports/go.mod (1)

12-13: Prefer go.work for local replaces; avoid placeholder version in released go.mod

The local replace is fine for dev, but keep transports/go.mod reproducible for CI/releases.

  • Move:
    • replace github.com/maximhq/bifrost/plugins/redis => ../plugins/redis
    • replace github.com/maximhq/bifrost/core => ../core
      into a repo-root go.work:
    go 1.24
    
    use (
      ./core
      ./plugins/redis
      ./transports
    )
    
    replace github.com/maximhq/bifrost/core => ./core
    replace github.com/maximhq/bifrost/plugins/redis => ./plugins/redis
    
  • In transports/go.mod, require a real tagged version of plugins/redis before merging. The pseudo-version will not resolve without the replace.

Also applies to: 20-23

ui/lib/api.ts (2)

234-241: Align GET/PUT Redis config response shapes

getRedisConfig() returns a bare RedisConfig, but updateRedisConfig() below expects a wrapped { config: RedisConfig }. This asymmetry will force callers to branch.

Choose one consistent shape for both endpoints and client types.


243-250: Client return type mismatches backend PUT /config/redis

Backend PUT responds with { status, message } (no config). Either adjust client typing or modify the handler to return the updated config.

Option A (update client now):

-  async updateRedisConfig(data: RedisConfig): ServiceResponse<{ config: RedisConfig }> {
+  async updateRedisConfig(data: RedisConfig): ServiceResponse<{ status: string; message: string }> {
     try {
       const response = await this.client.put('/config/redis', data)
       return [response.data, null]
     } catch (error) {
       return [null, this.getErrorMessage(error)]
     }
   }

Option B (keep client type; change backend): Include config in the handler JSON payload.

ui/lib/types/config.ts (1)

129-129: Document new public flag enable_caching

This is a public addition to CoreConfig. Please add/update docs and sample config JSON to include it and its default.

ui/app/config/page.tsx (1)

318-346: Let users edit Redis settings immediately after enabling caching

Form is rendered only when both persisted and current values are true. This blocks first-time editing post-toggle.

Render whenever the current toggled value is true:

-              {configInDB.enable_caching && config.enable_caching && (
+              {config.enable_caching && (
                 <div className="mt-4 space-y-4">
                   <Separator />
                   <RedisConfigForm />
                 </div>
               )}
transports/bifrost-http/main.go (1)

423-451: Avoid hard-coding the Redis cache context key

CacheKey: "request-cache-key" hard-codes the context lookup key. Derive this from a shared constant (exported by the redis plugin) or centralize it in a single source (e.g., lib/ctx.go) to avoid drift with header extraction logic.

If the intent is to map the x-bf-cache-key header into context, prefer using a shared exported const from the plugin to keep both sides in sync.

transports/bifrost-http/lib/store.go (3)

65-66: Add changelog/docs entry for EnableCaching

Public client config changed. Update docs and example config to include enable_caching (default false).


247-248: AutoMigrate on Redis table: consider start-up guard

Auto-migrating config_redis on every boot can cause unnecessary schema checks. Optional: gate AutoMigrate with a HasTable check and only migrate when needed.

 func (s *ConfigStore) autoMigrate() error {
-    return s.db.AutoMigrate(
+    // Always migrate core tables
+    if err := s.db.AutoMigrate(
         &DBConfigHash{},
         &DBProvider{},
         &DBKey{},
         &DBMCPClient{},
         &DBClientConfig{},
         &DBEnvKey{},
-        &DBRedisConfig{},
-    )
+    ); err != nil {
+        return err
+    }
+    // Migrate Redis table only if missing
+    if !s.db.Migrator().HasTable((&DBRedisConfig{}).TableName()) {
+        return s.db.AutoMigrate(&DBRedisConfig{})
+    }
+    return nil
 }

2104-2111: Make Redis config update atomic (transactional)

Delete-then-create without a transaction risks ending up with no config if create fails.

Wrap both in a single transaction:

 func (s *ConfigStore) UpdateRedisConfig(config *DBRedisConfig) error {
-    // Delete existing Redis config and create new one
-    if err := s.db.Session(&gorm.Session{AllowGlobalUpdate: true}).Delete(&DBRedisConfig{}).Error; err != nil {
-        return err
-    }
-    return s.db.Create(config).Error
+    return s.db.Transaction(func(tx *gorm.DB) error {
+        if err := tx.Session(&gorm.Session{AllowGlobalUpdate: true}).Delete(&DBRedisConfig{}).Error; err != nil {
+            return err
+        }
+        return tx.Create(config).Error
+    })
 }
transports/bifrost-http/lib/models.go (2)

142-143: Set explicit defaults for CacheByModel/Provider to preserve expected behavior

Without defaults, DB zero-values (false) will override plugin defaults when passed as pointers.

Apply this diff:

-    CacheByModel    bool      `gorm:"" json:"cache_by_model"`                      // Include model in cache key
-    CacheByProvider bool      `gorm:"" json:"cache_by_provider"`                   // Include provider in cache key
+    CacheByModel    bool      `gorm:"default:true" json:"cache_by_model"`          // Include model in cache key
+    CacheByProvider bool      `gorm:"default:true" json:"cache_by_provider"`       // Include provider in cache key

Note: Existing rows won’t backfill—add a one-time migration if needed.


133-146: Add a BeforeSave validation hook for DBRedisConfig

Validate essential fields to prevent persisting invalid rows (bad addr, non-positive TTL, out-of-range DB).

Add this hook (outside the shown range, near other hooks):

func (c *DBRedisConfig) BeforeSave(tx *gorm.DB) error {
	// Require non-empty addr; rudimentary format check
	if c.Addr == "" {
		return fmt.Errorf("redis addr is required")
	}
	if _, _, err := net.SplitHostPort(c.Addr); err != nil {
		return fmt.Errorf("invalid redis addr %q; expected host:port", c.Addr)
	}

	if c.TTLSeconds < 1 {
		return fmt.Errorf("ttl_seconds must be >= 1")
	}

	// Standard Redis supports 0..15 by default; allow override if your deployment differs
	if c.DB < 0 || c.DB > 15 {
		return fmt.Errorf("db must be between 0 and 15")
	}
	return nil
}

Remember to import "net" if not already present.

transports/bifrost-http/handlers/redis.go (4)

54-59: Validate addr format (host:port) before persisting

A malformed addr will break plugin init later. Validate up front.

 // Validate required fields
 if req.Addr == "" {
 	SendError(ctx, fasthttp.StatusBadRequest, "Redis address is required", h.logger)
 	return
 }
+
+// Validate addr format: require host:port
+if _, _, err := net.SplitHostPort(req.Addr); err != nil {
+	SendError(ctx, fasthttp.StatusBadRequest, "Invalid redis addr; expected host:port", h.logger)
+	return
+}

60-64: Reject non-positive TTL instead of silently coercing

Fail fast on invalid TTLs; makes issues visible to the user.

-// Validate TTL
-if req.TTLSeconds <= 0 {
-	req.TTLSeconds = 300 // Default to 5 minutes
-}
+// Validate TTL
+if req.TTLSeconds < 1 {
+	SendError(ctx, fasthttp.StatusBadRequest, "ttl_seconds must be >= 1", h.logger)
+	return
+}

65-69: Add DB index range check and preserve password when redacted placeholder is sent

  • Enforce standard Redis DB range (0..15).
  • If UI sends "********", keep existing password instead of overwriting with placeholder.
 // Update Redis configuration in database
- if err := h.store.UpdateRedisConfig(&req); err != nil {
+ // Validate DB number
+ if req.DB < 0 || req.DB > 15 {
+ 	SendError(ctx, fasthttp.StatusBadRequest, "Redis database number must be between 0 and 15", h.logger)
+ 	return
+ }
+
+ // If password is the redacted placeholder, preserve the stored value
+ if req.Password == "********" {
+ 	if current, err := h.store.GetRedisConfig(); err == nil && current != nil {
+ 		req.Password = current.Password
+ 	} else {
+ 		// fallback: don't write the placeholder
+ 		req.Password = ""
+ 	}
+ }
+
+ if err := h.store.UpdateRedisConfig(&req); err != nil {
 	SendError(ctx, fasthttp.StatusInternalServerError, fmt.Sprintf("Failed to update Redis config: %v", err), h.logger)
 	return
 }

71-79: Drop redundant SetStatusCode before SendJSON

SendJSON already sets status and headers.

 h.logger.Info("Redis configuration updated successfully")
 
-ctx.SetStatusCode(fasthttp.StatusOK)
 SendJSON(ctx, map[string]any{
 	"status":  "success",
 	"message": "Redis configuration updated successfully",
 	"config":  req,
 }, h.logger)
ui/components/config/redis-config-form.tsx (3)

28-33: Use portable timeout types for edge/browser runtimes

NodeJS.Timeout breaks without the "node" lib. Use ReturnType.

-  const addrTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-  const usernameTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-  const passwordTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-  const prefixTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
-  const ttlTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined)
+  const addrTimeoutRef = useRef<ReturnType<typeof setTimeout>>()
+  const usernameTimeoutRef = useRef<ReturnType<typeof setTimeout>>()
+  const passwordTimeoutRef = useRef<ReturnType<typeof setTimeout>>()
+  const prefixTimeoutRef = useRef<ReturnType<typeof setTimeout>>()
+  const ttlTimeoutRef = useRef<ReturnType<typeof setTimeout>>()

63-85: Fix state reversion and avoid clobbering concurrent edits

  • Store the previous config for proper rollback on error.
  • Use functional updates to prevent stale closures.
-  const updateConfig = async (updates: Partial<RedisConfig>) => {
-    const newConfig = { ...config, ...updates }
-    setConfig(newConfig)
+  const updateConfig = async (updates: Partial<RedisConfig>) => {
+    const previousConfig = config
+    const newConfig = { ...config, ...updates }
+    setConfig(prev => ({ ...prev, ...updates }))
 
     try {
       const [_, error] = await apiService.updateRedisConfig(newConfig)
       if (error) {
         if (error.includes('status code 405')) {
           toast.error('Please enable redis plugin and restart Bifrost.')
           return
         }
         toast.error(error)
         // Revert on error
-        setConfig(config)
+        setConfig(previousConfig)
       } else {
         toast.success('Redis configuration updated successfully')
       }
     } catch (error) {
       toast.error('Failed to update Redis configuration')
       // Revert on error
-      setConfig(config)
+      setConfig(previousConfig)
     }
   }

88-156: Reduce debounce duplication with a small helper

Five nearly identical handlers can be expressed via a generic builder without harming readability.

Example:

const createDebouncedHandler = <K extends keyof RedisConfig>(
  field: K,
  timeoutRef: React.MutableRefObject<ReturnType<typeof setTimeout> | undefined>
) => (value: RedisConfig[K]) => {
  setConfig(prev => ({ ...prev, [field]: value }))
  if (timeoutRef.current) clearTimeout(timeoutRef.current)
  timeoutRef.current = setTimeout(() => {
    updateConfig({ [field]: value } as Partial<RedisConfig>)
  }, 1000)
}

// Usage:
const handleAddrChange = createDebouncedHandler('addr', addrTimeoutRef)
const handleUsernameChange = createDebouncedHandler('username', usernameTimeoutRef)
const handlePasswordChange = createDebouncedHandler('password', passwordTimeoutRef)
const handlePrefixChange = createDebouncedHandler('prefix', prefixTimeoutRef)
const handleTtlChange = createDebouncedHandler('ttl_seconds', ttlTimeoutRef)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f7ce6c and 8431c85.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (13)
  • transports/bifrost-http/handlers/config.go (1 hunks)
  • transports/bifrost-http/handlers/redis.go (1 hunks)
  • transports/bifrost-http/lib/config.go (1 hunks)
  • transports/bifrost-http/lib/ctx.go (2 hunks)
  • transports/bifrost-http/lib/models.go (2 hunks)
  • transports/bifrost-http/lib/store.go (6 hunks)
  • transports/bifrost-http/main.go (3 hunks)
  • transports/go.mod (3 hunks)
  • ui/app/config/page.tsx (3 hunks)
  • ui/components/config/redis-config-form.tsx (1 hunks)
  • ui/lib/api.ts (2 hunks)
  • ui/lib/types/config.ts (1 hunks)
  • ui/lib/utils/validation.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (55)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T04:58:08.229Z
Learning: Pratham-Mishra04 prefers not to add logging or error handling for unreachable code paths in the Bifrost project. When provider types or similar entities are predefined in the system, defensive programming like logging in default cases is considered unnecessary overhead.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: docs/usage/go-package/mcp.md:408-412
Timestamp: 2025-07-01T12:40:08.576Z
Learning: Pratham-Mishra04 is okay with keeping bullet list formatting that uses colons after dashes in markdown documentation, even if it triggers linter warnings, preferring functionality over strict formatting rules.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: tests/core-providers/scenarios/chat_completion_stream.go:103-105
Timestamp: 2025-07-16T04:26:09.288Z
Learning: Pratham-Mishra04 prefers to keep test code simple when it serves its basic functional purpose. For tests that are meant to validate core functionality (like verifying streaming works), they consider hard-coded reasonable limits acceptable rather than making them configurable.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:880-910
Timestamp: 2025-07-08T17:14:21.544Z
Learning: Pratham-Mishra04 prefers resilient system design where missing environment variables for MCP connections should not cause complete system failure. The system should continue processing other MCP connections even when some fail, maintaining partial functionality rather than implementing fail-fast behavior.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#138
File: transports/README.md:26-28
Timestamp: 2025-07-01T12:45:06.906Z
Learning: Pratham-Mishra04 prefers keeping documentation examples simple and concise, trusting users to handle production-specific considerations like version pinning themselves rather than cluttering examples with additional notes.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:1081-1098
Timestamp: 2025-07-08T17:16:50.811Z
Learning: Pratham-Mishra04 prefers practical redaction approaches over theoretical security improvements when the threat model is low-risk, such as admin-only interfaces in the Bifrost project. Fixed-length redaction is acceptable when only trusted administrators will see the redacted values.
📚 Learning: 2025-08-07T13:33:18.094Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: plugins/jsonparser/main.go:0-0
Timestamp: 2025-08-07T13:33:18.094Z
Learning: In the JSON Parser plugin (plugins/jsonparser/main.go), the context key for enabling per-request JSON parsing is "enable-streaming-json-parser" (defined as EnableStreamingJSONParser constant), which is distinct from the plugin name constant to avoid confusion between plugin identification and per-request activation control.

Applied to files:

  • transports/bifrost-http/lib/config.go
📚 Learning: 2025-08-03T20:36:21.906Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:183-191
Timestamp: 2025-08-03T20:36:21.906Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers the current string matching approach for determining rate limit violation decision types (DecisionTokenLimited vs DecisionRequestLimited) adequate and not important enough to refactor with more robust explicit checks, preferring functional simplicity over theoretical robustness improvements.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-03T20:50:48.247Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/tracker.go:146-146
Timestamp: 2025-08-03T20:50:48.247Z
Learning: In the Bifrost governance tracker (transports/bifrost-http/plugins/governance/tracker.go), Pratham-Mishra04 implements database persistence for reset operations using arrays to collect reset entities (resetRateLimits and resetBudgets) and then batch persist them using t.db.Save() with proper error aggregation in the PerformStartupResets method.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called in two specific scenarios: when ErrorDetails is present (error case) and when isFinalChunk is true (end of streaming). It is not called repeatedly during streaming, making performance optimization with caching unnecessary.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-08-01T16:25:49.937Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#174
File: transports/bifrost-http/plugins/logging/operations.go:113-114
Timestamp: 2025-08-01T16:25:49.937Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/operations.go), the calculateLatency method is only called at final chunk of streaming responses and when errors are returned, not repeatedly during streaming. This makes the performance impact of the database query minimal and doesn't warrant optimization with in-memory caching.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-07-18T10:52:24.942Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#159
File: docs/usage/http-transport/README.md:0-0
Timestamp: 2025-07-18T10:52:24.942Z
Learning: In the Bifrost project, the npx wrapper (`npx -y bifrostlatest -port 8080`) correctly handles single-dash long flags like `-port` without requiring double dashes or the `--` separator. The implementation properly forwards flags to the underlying Go binary.

Applied to files:

  • ui/lib/utils/validation.ts
📚 Learning: 2025-06-20T16:21:18.912Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/main.go
📚 Learning: 2025-06-27T17:07:39.462Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#135
File: docs/core-package.md:105-116
Timestamp: 2025-06-27T17:07:39.462Z
Learning: In Go, when importing a package, the identifier used to access it is determined by the `package` declaration in the Go source files, not the directory name. For the Bifrost project, the core directory files declare `package bifrost`, so importing `"github.com/maximhq/bifrost/core"` automatically makes it accessible as `bifrost.Init`, `bifrost.Ptr`, etc., without needing an explicit alias.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-06-16T03:55:16.949Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T03:55:16.949Z
Learning: Go 1.24 was released in February 2025 and is stable and available for use in go.mod files.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-04T03:57:50.981Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-07-08T18:12:13.590Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: docs/contributing/README.md:22-27
Timestamp: 2025-07-08T18:12:13.590Z
Learning: In the Bifrost project, the tests directory structure has `tests/core-providers/` and `tests/transports-integrations/` as sibling directories. From `tests/core-providers/`, the correct relative path to reach `tests/transports-integrations/` is `../transports-integrations/`, not `../../tests/transports-integrations/`.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-16T03:54:48.005Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:38-38
Timestamp: 2025-06-16T03:54:48.005Z
Learning: The `core-providers-test` module in `tests/core-providers/` is an internal testing module that will never be consumed as a dependency by external projects, so the replace directive pointing to `../../core` is acceptable for local development and testing purposes.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-17T14:04:21.104Z
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-03T20:17:11.876Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/plugins/governance/resolver.go:68-74
Timestamp: 2025-08-03T20:17:11.876Z
Learning: In the Bifrost governance plugin (transports/bifrost-http/plugins/governance/resolver.go), Pratham-Mishra04 considers nil validation in constructors like NewBudgetResolver unnecessary because the parameters will never be nil in practice. This aligns with their general preference to avoid defensive programming for scenarios they consider unreachable.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-19T16:57:25.177Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:30-36
Timestamp: 2025-06-19T16:57:25.177Z
Learning: In the bifrost repository, Pratham-Mishra04 prefers to keep GitHub Actions workflows lean and trusts their controlled tagging process for core releases, avoiding unnecessary validation steps that they consider overkill.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-09T14:03:34.227Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:0-0
Timestamp: 2025-06-09T14:03:34.227Z
Learning: In the Bifrost HTTP transport layer (transports/bifrost-http/integrations/), request validation like checking for empty messages should be handled by the provider rather than at the transport layer. The transport layer should forward requests to Bifrost core/providers for validation.

Applied to files:

  • transports/go.mod
  • ui/lib/api.ts
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-16T04:29:53.409Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use `package main` for test files in the tests/core-providers directory rather than more descriptive package names like `coreproviders_test`.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-07-08T18:09:32.147Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-07-08T18:09:32.147Z
Learning: In the Bifrost HTTP transport logging plugin (transports/bifrost-http/plugins/logging/main.go), the DatabasePath is explicitly set from main.go via a config struct pointing to appDir/logs, so the default value in the NewLoggerPlugin constructor is only used for standalone plugin usage and doesn't affect the main application flow.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-16T04:13:55.437Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-08-08T07:07:08.801Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/providers/openai.go:459-459
Timestamp: 2025-08-08T07:07:08.801Z
Learning: In core/providers/utils.go, processAndSendResponse and processAndSendError now take exactly five parameters: (ctx, postHookRunner, response|err, responseChan, logger). All providers (OpenAI, Anthropic, Cohere, Bedrock) must call these with the logger as the fifth argument; a repo-wide check in PR maximhq/bifrost#219 confirmed this is already consistent.

Applied to files:

  • transports/go.mod
  • ui/lib/api.ts
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-15T14:24:49.882Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-16T04:12:05.427Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and always returns a pointer to a valid context. It starts with context.Background() and only adds values to it, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-08-03T20:11:26.945Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:220-220
Timestamp: 2025-08-03T20:11:26.945Z
Learning: In the Bifrost HTTP transport handlers using fasthttp router, path parameters extracted via ctx.UserValue() can be safely type-asserted to string without additional checks because the router guarantees their presence and type when routes match. Pratham-Mishra04 confirmed that defensive type assertion checks are unnecessary overhead in this context.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-04T22:19:12.975Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#212
File: transports/bifrost-http/integrations/openai/types.go:250-288
Timestamp: 2025-08-04T22:19:12.975Z
Learning: In transports/bifrost-http/integrations/openai/types.go, the ConvertToBifrostRequest function for OpenAIEmbeddingRequest handles []interface{} input without validation for non-string elements because Pratham-Mishra04 confirmed this case will never happen in practice, consistent with their preference to avoid defensive programming for unreachable scenarios.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-10T13:51:52.859Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:140-146
Timestamp: 2025-06-10T13:51:52.859Z
Learning: In Bifrost core v1.0.9, ImageContent.Type was a pointer type (*string accessed via bifrost.Ptr), but in v1.0.10 it was changed to a value type (ImageContentType). When reviewing code, check the core version being used to determine the correct assignment pattern.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-10T11:19:29.604Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
📚 Learning: 2025-08-08T14:12:14.754Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#238
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-08-08T14:12:14.754Z
Learning: In transports/bifrost-http/plugins/logging/main.go, stream accumulator cleanup is intended to use a 5-minute TTL (consistent with processing logs). Comments should state “older than 5 minutes,” and using central constants (processingLogTTL, streamAccumulatorTTL) helps avoid future mismatches.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-04T09:22:18.123Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/bifrost.go:46-49
Timestamp: 2025-06-04T09:22:18.123Z
Learning: In core/schemas/bifrost.go, the RequestInput struct uses ChatCompletionInput *[]BifrostMessage (pointer-to-slice) rather than []BifrostMessage to properly represent union type semantics. For text completion requests, ChatCompletionInput should be nil to indicate "no chat payload at all", while for chat completion requests it should be non-nil (even if empty slice). This distinguishes between different request types rather than just empty vs non-empty chat messages.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-26T07:37:24.385Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#131
File: transports/bifrost-http/lib/config.go:35-68
Timestamp: 2025-06-26T07:37:24.385Z
Learning: In the Bifrost project's MCP configuration handling, empty environment variables should be treated as missing/invalid rather than as valid empty values. The current implementation using `os.Getenv(envKey); envValue != ""` to check for non-empty values is the desired behavior.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-08-08T15:31:28.737Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#238
File: transports/bifrost-http/plugins/logging/main.go:687-703
Timestamp: 2025-08-08T15:31:28.737Z
Learning: In transports/bifrost-http/plugins/logging/main.go, TejasGhatte prefers keeping separate helpers for streaming detection: isStreamingResponse (any streaming) and isTextStreamingResponse (text-only). Avoid parameterizing/merging them, as that adds code and reduces clarity.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-05T20:43:59.593Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#214
File: core/providers/azure.go:49-50
Timestamp: 2025-08-05T20:43:59.593Z
Learning: In core/providers/azure.go, the azureTextCompletionResponsePool should use AzureTextResponse type, not schemas.BifrostResponse, to maintain consistency with the acquireAzureTextResponse() and releaseAzureTextResponse() functions that work with *AzureTextResponse objects.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-07-10T13:44:14.518Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.518Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-10T13:44:39.237Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:16-18
Timestamp: 2025-07-10T13:44:39.237Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers to ignore JSON marshaling errors when storing log entries because logging is not critical for their use case and they are certain the marshaling operations won't fail.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-10T13:44:23.297Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#152
File: transports/bifrost-http/plugins/logging/utils.go:94-111
Timestamp: 2025-07-10T13:44:23.297Z
Learning: Pratham-Mishra04 prefers not to add error handling for JSON marshaling operations in the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go) because logging is not critical functionality and the structured schema data being marshaled is unlikely to fail. They accept the risk of not handling json.Marshal errors in logging contexts to keep the code simple.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-08T06:51:27.123Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#219
File: core/schemas/bifrost.go:686-692
Timestamp: 2025-08-08T06:51:27.123Z
Learning: In core/schemas/bifrost.go, BifrostError.StreamControl is intended for plugin developers only and is excluded from JSON serialization; its presence controls streaming behavior internally.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-08-03T20:53:08.832Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: ui/app/governance/page.tsx:29-60
Timestamp: 2025-08-03T20:53:08.832Z
Learning: In the Bifrost governance page (ui/app/governance/page.tsx), Pratham-Mishra04 intentionally keeps the loading state active when critical errors occur (such as governance being disabled or core config failures) to prevent showing a broken or incomplete governance interface to users. This is a deliberate UX design choice to avoid presenting non-functional page states.

Applied to files:

  • ui/app/config/page.tsx
  • ui/components/config/redis-config-form.tsx
📚 Learning: 2025-07-08T18:20:24.086Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-07-08T17:31:44.662Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#145
File: transports/bifrost-http/main.go:124-128
Timestamp: 2025-07-08T17:31:44.662Z
Learning: Pratham-Mishra04 prefers to keep the CORS middleware simple in the Bifrost HTTP transport (transports/bifrost-http/main.go) rather than adding port validation for localhost origins, considering the current implementation sufficient for the intended use case.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-09T16:35:26.914Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#64
File: transports/bifrost-http/integrations/genai/types.go:273-313
Timestamp: 2025-06-09T16:35:26.914Z
Learning: In convertGenerationConfigToParams method in transports/bifrost-http/integrations/genai/types.go, pre-allocating the ExtraParams map is preferred over lazy allocation because the method has multiple potential ExtraParams assignments, making the computational overhead of conditional checks exceed the memory savings of an empty map.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-06-15T16:07:53.140Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#85
File: core/providers/anthropic.go:150-156
Timestamp: 2025-06-15T16:07:53.140Z
Learning: In the Bifrost codebase, constructor functions are allowed to mutate input ProviderConfig objects in-place (e.g., setting default BaseURL values and trimming trailing slashes). This pattern is acceptable and doesn't need to be flagged as a code review issue.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • ui/components/config/redis-config-form.tsx
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-08-02T13:02:13.642Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#174
File: transports/bifrost-http/lib/models.go:251-254
Timestamp: 2025-08-02T13:02:13.642Z
Learning: In the Bifrost project's database models (transports/bifrost-http/lib/models.go), Pratham-Mishra04 considers meta config types to be predefined and expects that unknown meta config types will never occur in practice. The current implementation that returns nil for unknown types in the AfterFind hook is intentional, as defensive programming for this scenario is considered unnecessary.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-06-16T14:44:42.893Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:151-155
Timestamp: 2025-06-16T14:44:42.893Z
Learning: In transports/bifrost-http/integrations/genai/types.go, when SystemInstruction has an empty role, the user prefers to return a validation error rather than setting a default role value. This represents a design preference for strict input validation over silent error correction.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-03T20:28:00.857Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:1038-1049
Timestamp: 2025-08-03T20:28:00.857Z
Learning: In the Bifrost governance handler (transports/bifrost-http/handlers/governance.go), Pratham-Mishra04 is comfortable with exposing full error details including err.Error() in production responses for governance API endpoints, prioritizing simplicity over potential information disclosure concerns.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-15T14:34:29.401Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: core/providers/ollama.go:59-61
Timestamp: 2025-06-15T14:34:29.401Z
Learning: The `CheckAndSetDefaults()` method on `*schemas.ProviderConfig` in Go does not return any error - it has a void return type and only sets default values on the configuration struct.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-06-10T11:19:29.604Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-09T06:55:22.017Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T06:55:22.017Z
Learning: In the Bifrost project's ConfigStore, different functions handle sensitive values differently based on their use cases. The `restoreMetaConfigEnvVars` function preserves actual values without redaction when writing config back to file for round-trip fidelity, while `GetProviderConfigRedacted` redacts sensitive values for external API responses for security purposes. This different handling is intentional and based on the specific context where each function is used.

Applied to files:

  • transports/bifrost-http/lib/store.go
  • ui/components/config/redis-config-form.tsx
📚 Learning: 2025-07-08T16:40:59.098Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:286-292
Timestamp: 2025-07-08T16:40:59.098Z
Learning: Pratham-Mishra04 prefers to keep simpler error handling patterns when errors are unlikely to occur in practice, such as safety checks in BadgerDB iterations where item.Value() is called on valid items. The user considers the overhead of explicit error handling not worth it in such scenarios.

Applied to files:

  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-29T16:10:52.088Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#196
File: core/providers/openai.go:180-183
Timestamp: 2025-07-29T16:10:52.088Z
Learning: In the Bifrost provider architecture, `handleProviderResponse` is a utility function that only parses and returns raw response data when the `sendBackRawResponse` flag is true. It's the responsibility of each individual provider (OpenAI, Anthropic, etc.) to conditionally set `response.ExtraFields.RawResponse` using the returned raw response data based on their `sendBackRawResponse` flag. This represents a separation of concerns where the utility handles parsing and the provider handles response object construction.

Applied to files:

  • ui/lib/api.ts
📚 Learning: 2025-06-16T14:50:46.859Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#89
File: transports/bifrost-http/integrations/genai/types.go:22-56
Timestamp: 2025-06-16T14:50:46.859Z
Learning: In the Google GenAI integration at transports/bifrost-http/integrations/genai/types.go, the manual URL-safe base64 decoding implementation (converting - to +, _ to /, and adding padding) is required because base64.RawURLEncoding.DecodeString fails for the specific url encoded bytes format being handled.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-06-16T04:13:42.755Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-07-08T16:48:25.386Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#144
File: transports/bifrost-http/plugins/logging/utils.go:509-514
Timestamp: 2025-07-08T16:48:25.386Z
Learning: In the Bifrost logging system (transports/bifrost-http/plugins/logging/utils.go), the Content field in message structures is a struct value type, not a pointer, so it will never be nil and doesn't require nil checks. However, ContentStr within Content is a pointer and should be checked for nil.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
📚 Learning: 2025-08-04T11:27:45.020Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#208
File: transports/bifrost-http/handlers/completions.go:221-225
Timestamp: 2025-08-04T11:27:45.020Z
Learning: In transports/bifrost-http/handlers/completions.go, Pratham-Mishra04 intentionally delegates file size validation for audio transcription uploads to the downstream API/provider rather than enforcing MaxFileSize limits at the HTTP transport layer. This follows their architectural preference of keeping the transport layer simple and letting providers handle domain-specific validation concerns, even for potential memory exhaustion scenarios.

Applied to files:

  • transports/bifrost-http/handlers/redis.go
🧬 Code Graph Analysis (6)
ui/app/config/page.tsx (3)
ui/components/ui/switch.tsx (1)
  • Switch (37-37)
ui/components/ui/separator.tsx (1)
  • Separator (43-43)
ui/components/config/redis-config-form.tsx (1)
  • RedisConfigForm (23-261)
transports/bifrost-http/main.go (3)
transports/bifrost-http/handlers/redis.go (2)
  • RedisHandler (15-18)
  • NewRedisHandler (21-26)
transports/bifrost-http/lib/config.go (1)
  • ClientConfig (11-20)
plugins/redis/main.go (2)
  • RedisPluginConfig (22-48)
  • NewRedisPlugin (83-146)
transports/bifrost-http/lib/store.go (2)
transports/bifrost-http/lib/models.go (2)
  • DBRedisConfig (134-146)
  • DBRedisConfig (155-155)
transports/bifrost-http/lib/config.go (1)
  • ClientConfig (11-20)
ui/components/config/redis-config-form.tsx (6)
ui/lib/types/config.ts (1)
  • RedisConfig (134-146)
ui/lib/api.ts (1)
  • apiService (506-506)
ui/components/ui/card.tsx (2)
  • Card (50-50)
  • CardContent (50-50)
ui/components/ui/label.tsx (1)
  • Label (24-24)
ui/components/ui/input.tsx (1)
  • Input (7-22)
ui/components/ui/switch.tsx (1)
  • Switch (37-37)
ui/lib/api.ts (1)
ui/lib/types/config.ts (1)
  • RedisConfig (134-146)
transports/bifrost-http/handlers/redis.go (4)
transports/bifrost-http/lib/store.go (1)
  • ConfigStore (32-47)
core/schemas/logger.go (1)
  • Logger (18-35)
transports/bifrost-http/handlers/utils.go (2)
  • SendError (27-36)
  • SendJSON (16-24)
transports/bifrost-http/lib/models.go (2)
  • DBRedisConfig (134-146)
  • DBRedisConfig (155-155)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
🔇 Additional comments (8)
transports/bifrost-http/lib/config.go (1)

18-18: LGTM: EnableCaching field addition is consistent

Naming and JSON tag align with existing ClientConfig style. No concerns here.

transports/go.mod (1)

46-46: Sanity check indirect deps

New indirects (go-rendezvous, go-redis/v9) are expected for Redis. No issues; just ensure they remain indirect in go.mod and are pruned by go mod tidy post-tagging.

Also applies to: 66-66

ui/lib/types/config.ts (1)

133-146: RedisConfig type looks correct

Field names and casing match backend JSON (addr, db, ttl_seconds, prefix, cache_by_model/provider). LGTM.

transports/bifrost-http/lib/store.go (3)

311-320: DB → in-memory mapping includes EnableCaching

Mapping EnableCaching from DBClientConfig to ClientConfig is correct. LGTM.


699-702: Persisting EnableCaching

Saving the new flag looks consistent with other fields. LGTM.


2085-2103: Graceful default for missing Redis config

Returning sensible defaults on ErrRecordNotFound avoids first-boot crashes. Good call.

transports/bifrost-http/lib/models.go (2)

113-113: EnableCaching flag addition looks good

Field naming and placement are consistent with existing client config booleans.


155-155: TableName for config_redis is appropriate

Consistent with naming in this file. No issues.

Comment thread transports/bifrost-http/handlers/redis.go Outdated
Comment thread transports/bifrost-http/handlers/redis.go Outdated
Comment thread ui/components/config/redis-config-form.tsx
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-11-_plugins_feat_redis_caching_plugin_added branch from 8efa472 to e524738 Compare August 11, 2025 07:31
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 08-08-feat_redis_plugin_added_to_transport branch from 8431c85 to ad8f0c1 Compare August 11, 2025 07:31
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🔭 Outside diff range comments (1)
transports/bifrost-http/main.go (1)

87-88: Fix embed directive path to UI assets

The embed pattern all:ui is evaluated relative to transports/bifrost-http/main.go, so it’s looking for transports/bifrost-http/ui (which doesn’t exist). Update the directive to point at the repo-root ui folder:

File: transports/bifrost-http/main.go
Lines: 87

- //go:embed all:ui
+ //go:embed all:../../ui
  var uiContent embed.FS

This ensures Go embeds the actual ui/ directory under the project root.

♻️ Duplicate comments (24)
ui/lib/utils/validation.ts (1)

313-343: Fix parseInt validation and consider IPv6 support.

The current implementation has the same issues identified in previous reviews: parseInt accepts partial numeric strings like "6379abc", and IPv6 addresses in bracket notation are not supported.

ui/lib/types/config.ts (1)

133-147: Add password redaction support to RedisConfig.

The RedisConfig interface lacks a way to indicate if a password is set without exposing the actual password value, which aligns with the project's practical redaction approach for admin-only UIs.

transports/go.mod (1)

20-22: Move local replace directives to go.work.

The local replace directives should be moved to a top-level go.work file to keep the module's go.mod clean and ensure reproducible builds for CI/release.

transports/bifrost-http/handlers/config.go (1)

88-88: EnableCaching toggle requires service restart to take effect.

As identified in previous reviews, toggling EnableCaching only persists the setting but doesn't dynamically start/stop the Redis plugin, which is initialized once at startup.

transports/bifrost-http/lib/ctx.go (1)

116-127: Use exported constants from Redis plugin to avoid magic string drift.

As identified in previous reviews, using string literals "request-cache-key" and "request-cache-ttl" creates risk of magic string drift. These should be exported constants from the Redis plugin.

ui/app/config/page.tsx (1)

337-342: Allow Redis configuration immediately after enabling caching.

As identified in previous reviews, the current condition prevents users from configuring Redis settings immediately after toggling caching on, creating unnecessary two-step workflow with restart requirement.

ui/lib/api.ts (2)

234-241: Avoid returning raw Redis password to the UI

To reduce secret exposure risk in admin UI, prefer redacting password in GET and exposing a password_set boolean. Only send a new password on PUT when changed.


243-250: Fix response type mismatch with backend

The backend's UpdateRedisConfig handler returns { status: string, message: string } but the client expects { config: RedisConfig }. Update the return type to match the actual backend response.

ui/components/config/redis-config-form.tsx (5)

28-33: Type for timeout refs breaks on edge runtimes

NodeJS.Timeout is only present when the "node" lib is included. Use portable type:

const addrTimeoutRef = useRef<ReturnType<typeof setTimeout>>();

63-85: Fix state reversion logic

The error handling attempts to revert state by setting setConfig(config), but config has already been modified by the state update on line 65. Store the previous config before updating to properly revert changes.


88-156: Extract common debounce logic to reduce code duplication

The debounced update handlers contain repetitive code that could be abstracted into a reusable function.


181-187: Constrain DB number to valid range

Add max="15" attribute and clamp the value to 0-15 range to match Redis's valid database numbers.

   <Input
     id="db"
     type="number"
     min="0"
+    max="15"
     value={config.db}
-    onChange={(e) => updateConfig({ db: parseInt(e.target.value) || 0 })}
+    onChange={(e) => {
+      const val = parseInt(e.target.value, 10)
+      const db = Number.isFinite(val) ? Math.min(15, Math.max(0, val)) : 0
+      updateConfig({ db })
+    }}
   />

219-225: Guard against NaN in TTL input

When the TTL input is cleared, parseInt('') becomes NaN. Add proper validation:

-onChange={(e) => handleTtlChange(parseInt(e.target.value) || 300)}
+onChange={(e) => {
+  const val = parseInt(e.target.value, 10)
+  handleTtlChange(Number.isFinite(val) && val > 0 ? val : 300)
+}}
transports/bifrost-http/main.go (2)

431-436: Startup fails when Redis config row is absent

If EnableCaching is toggled on but no row exists yet (first-time enable), store.GetRedisConfig() may return gorm.ErrRecordNotFound causing the service to crash. Initialize with defaults instead.


444-445: Avoid hard-coding the cache key

CacheKey: "request-cache-key" prevents per-request cache key control via the x-bf-cache-key header. Consider deriving it from the request context instead.

transports/bifrost-http/lib/models.go (1)

134-146: Add validation hooks and defaults for Redis config

  1. Add a BeforeSave hook to validate Addr format and TTLSeconds > 0
  2. Set GORM defaults for boolean fields to preserve expected behavior:
-    CacheByModel    bool      `gorm:"" json:"cache_by_model"`
-    CacheByProvider bool      `gorm:"" json:"cache_by_provider"`
+    CacheByModel    bool      `gorm:"default:true" json:"cache_by_model"`
+    CacheByProvider bool      `gorm:"default:true" json:"cache_by_provider"`
transports/bifrost-http/lib/store.go (4)

65-66: Document new public flag EnableCaching in changelog and samples

Please add a changelog/docs note and update sample configs for the new ClientConfig field EnableCaching.


247-248: Auto-migrate on Redis table can cause startup schema churn; guard it

Consider guarding AutoMigrate for DBRedisConfig to avoid unnecessary schema diffs/locks on every start.

 func (s *ConfigStore) autoMigrate() error {
-	return s.db.AutoMigrate(
+	// Optional: guard Redis config table to reduce startup churn on large DBs
+	if s.db.Migrator().HasTable((&DBRedisConfig{}).TableName()) {
+		// table exists; migrate only if needed or skip
+	}
+	return s.db.AutoMigrate(
 		&DBConfigHash{},
 		&DBProvider{},
 		&DBKey{},
 		&DBMCPClient{},
 		&DBClientConfig{},
 		&DBEnvKey{},
 		&DBRedisConfig{},
 	)
 }

2085-2102: Make Redis config retrieval deterministic and/or enforce singleton

First() is non-deterministic if duplicates slip in. Prefer ordering or enforce a single-row invariant.

Option A (deterministic fetch of latest):

- if err := s.db.First(&redisConfig).Error; err != nil {
+ if err := s.db.Order("updated_at DESC").Limit(1).Take(&redisConfig).Error; err != nil {

Option B (singleton by fixed ID, e.g., ID=1):

  • Store row with ID=1 always; fetch by primary key.
  • Add a unique constraint in migrations.

Either approach avoids arbitrary selection and surprises.


2105-2111: Update is not atomic; use a transaction and prefer UPDATE/UPSERT over delete-all

Deleting all rows before insert risks ending up with no Redis config if Create fails. Wrap in a transaction and avoid full delete where possible.

Minimal fix (transaction):

 func (s *ConfigStore) UpdateRedisConfig(config *DBRedisConfig) error {
-	// Delete existing Redis config and create new one
-	if err := s.db.Session(&gorm.Session{AllowGlobalUpdate: true}).Delete(&DBRedisConfig{}).Error; err != nil {
-		return err
-	}
-	return s.db.Create(config).Error
+	return s.db.Transaction(func(tx *gorm.DB) error {
+		if err := tx.Session(&gorm.Session{AllowGlobalUpdate: true}).Delete(&DBRedisConfig{}).Error; err != nil {
+			return err
+		}
+		return tx.Create(config).Error
+	})
 }

Better: enforce a singleton (ID=1) and upsert, avoiding delete+create entirely.

transports/bifrost-http/handlers/redis.go (4)

3-12: Add imports for robust validation and path unescaping

To validate addr with net.SplitHostPort and to unescape route params, import net and net/url.

 import (
 	"encoding/json"
 	"fmt"
+	"net"
+	"net/url"

 	"github.com/fasthttp/router"
 	"github.com/maximhq/bifrost/core/schemas"
 	"github.com/maximhq/bifrost/plugins/redis"
 	"github.com/maximhq/bifrost/transports/bifrost-http/lib"
 	"github.com/valyala/fasthttp"
 )

38-47: Redact password in GET response to avoid secret leakage

Returning plaintext Password is risky. Redact or omit it in responses.

-	SendJSON(ctx, config, h.logger)
+	// Redact password before responding
+	if config != nil && config.Password != "" {
+		resp := *config // shallow copy
+		resp.Password = "********"
+		SendJSON(ctx, resp, h.logger)
+		return
+	}
+	SendJSON(ctx, config, h.logger)

58-67: Strengthen input validation: addr format, TTL minimum, DB range

  • Validate addr with net.SplitHostPort
  • Reject TTLSeconds < 1
  • Validate DB in 0..15 (typical default range)
 	// Validate required fields
 	if req.Addr == "" {
 		SendError(ctx, fasthttp.StatusBadRequest, "Redis address is required", h.logger)
 		return
 	}
 
-	// Validate TTL
-	if req.TTLSeconds <= 0 {
-		req.TTLSeconds = 300 // Default to 5 minutes
-	}
+	// Validate addr format
+	if _, _, err := net.SplitHostPort(req.Addr); err != nil {
+		SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("Invalid Redis address (host:port): %v", err), h.logger)
+		return
+	}
+
+	// Validate TTL
+	if req.TTLSeconds < 1 {
+		SendError(ctx, fasthttp.StatusBadRequest, "TTLSeconds must be >= 1", h.logger)
+		return
+	}
+
+	// Validate DB index (common default range)
+	if req.DB < 0 || req.DB > 15 {
+		SendError(ctx, fasthttp.StatusBadRequest, "Redis database number must be between 0 and 15", h.logger)
+		return
+	}

77-83: Redundant status code set before SendJSON

SendJSON already sets 200 and headers. Drop the explicit SetStatusCode.

-	ctx.SetStatusCode(fasthttp.StatusOK)
 	SendJSON(ctx, map[string]any{
 		"status":  "success",
 		"message": "Redis configuration updated successfully",
 		"config":  req,
 	}, h.logger)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8431c85 and ad8f0c1.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (15)
  • docs/usage/http-transport/endpoints.md (1 hunks)
  • plugins/redis/README.md (1 hunks)
  • transports/bifrost-http/handlers/config.go (1 hunks)
  • transports/bifrost-http/handlers/redis.go (1 hunks)
  • transports/bifrost-http/lib/config.go (1 hunks)
  • transports/bifrost-http/lib/ctx.go (2 hunks)
  • transports/bifrost-http/lib/models.go (2 hunks)
  • transports/bifrost-http/lib/store.go (6 hunks)
  • transports/bifrost-http/main.go (3 hunks)
  • transports/go.mod (3 hunks)
  • ui/app/config/page.tsx (3 hunks)
  • ui/components/config/redis-config-form.tsx (1 hunks)
  • ui/lib/api.ts (2 hunks)
  • ui/lib/types/config.ts (1 hunks)
  • ui/lib/utils/validation.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
📚 Learning: 2025-06-17T14:04:21.104Z
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-20T16:21:18.912Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-07-08T18:20:24.086Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.

Applied to files:

  • transports/bifrost-http/main.go
🧬 Code Graph Analysis (6)
ui/lib/api.ts (1)
ui/lib/types/config.ts (1)
  • RedisConfig (134-146)
ui/components/config/redis-config-form.tsx (3)
ui/lib/types/config.ts (1)
  • RedisConfig (134-146)
ui/lib/api.ts (1)
  • apiService (506-506)
ui/components/ui/input.tsx (1)
  • Input (7-22)
transports/bifrost-http/lib/store.go (2)
transports/bifrost-http/lib/models.go (2)
  • DBRedisConfig (134-146)
  • DBRedisConfig (155-155)
transports/bifrost-http/lib/config.go (1)
  • ClientConfig (11-20)
transports/bifrost-http/handlers/redis.go (4)
transports/bifrost-http/lib/store.go (1)
  • ConfigStore (32-47)
core/schemas/logger.go (1)
  • Logger (18-39)
transports/bifrost-http/handlers/utils.go (2)
  • SendError (27-36)
  • SendJSON (16-24)
transports/bifrost-http/lib/models.go (2)
  • DBRedisConfig (134-146)
  • DBRedisConfig (155-155)
transports/bifrost-http/lib/ctx.go (4)
core/providers/utils.go (1)
  • ContextKey (63-63)
transports/bifrost-http/plugins/logging/main.go (1)
  • ContextKey (22-22)
plugins/maxim/main.go (1)
  • ContextKey (54-54)
transports/bifrost-http/plugins/governance/models.go (1)
  • ParseDuration (222-256)
transports/bifrost-http/main.go (3)
transports/bifrost-http/handlers/redis.go (2)
  • RedisHandler (16-20)
  • NewRedisHandler (23-29)
transports/bifrost-http/lib/config.go (1)
  • ClientConfig (11-20)
plugins/redis/main.go (2)
  • RedisPluginConfig (22-49)
  • NewRedisPlugin (84-147)
🪛 LanguageTool
docs/usage/http-transport/endpoints.md

[grammar] ~414-~414: Use correct spacing
Context: ..._limit"} 23 ``` --- ## 🧾 Redis Cache Management Bifrost provides built-in Redis caching ...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~416-~416: Use correct spacing
Context: ...s to improve performance and reduce API costs. ### Cache Control Headers Add these heade...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~418-~418: Use correct spacing
Context: ... reduce API costs. ### Cache Control Headers Add these headers to your requests to co...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~420-~420: There might be a mistake here.
Context: ...ers to your requests to control caching behavior: | Header | Type | Description | Example...

(QB_NEW_EN_OTHER)


[grammar] ~425-~425: There might be a mistake here.
Context: ...tion for cache entry | "30s", "5m", "1h" | cURL Example with Caching: ```bash c...

(QB_NEW_EN_OTHER)


[grammar] ~427-~427: There might be a mistake here.
Context: ..., "5m", "1h" | cURL Example with Caching: bash curl -X POST http://localhost:8080/v1/chat/completions \ -H "Content-Type: application/json" \ -H "x-bf-cache-key: user-session-abc123" \ -H "x-bf-cache-ttl: 10m" \ -d '{ "model": "openai/gpt-4o-mini", "messages": [ {"role": "user", "content": "What is the capital of France?"} ] }' Cache Behavior: - **Fir...

(QB_NEW_EN_OTHER)


[grammar] ~442-~442: There might be a mistake here.
Context: ...al of France?"} ] }' ``` Cache Behavior: - First request: Goes to the AI provide...

(QB_NEW_EN_OTHER)


[grammar] ~444-~444: There might be a mistake here.
Context: ...** - First request: Goes to the AI provider, response is cached - **Subsequent ident...

(QB_NEW_EN_OTHER)


[grammar] ~444-~444: There might be a mistake here.
Context: ...*: Goes to the AI provider, response is cached - Subsequent identical requests: Served ...

(QB_NEW_EN_OTHER)


[grammar] ~445-~445: There might be a mistake here.
Context: ...requests**: Served instantly from Redis cache - Cache hits: Include `bifrost_cached: t...

(QB_NEW_EN_OTHER)


[grammar] ~446-~446: There might be a mistake here.
Context: ...lude bifrost_cached: true in response metadata - Streaming responses: Cached and recons...

(QB_NEW_EN_OTHER)


[grammar] ~447-~447: There might be a mistake here.
Context: ...es**: Cached and reconstructed chunk by chunk ### DELETE /api/redis/{key} Delete a spec...

(QB_NEW_EN_OTHER)


[grammar] ~449-~449: Use correct spacing
Context: ...onstructed chunk by chunk ### DELETE /api/redis/{key} Delete a specific cache entry from Redis...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~451-~451: Use correct spacing
Context: ...}** Delete a specific cache entry from Redis. URL Parameters: - key (required): T...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~453-~453: There might be a mistake here.
Context: ...specific cache entry from Redis. URL Parameters: - key (required): The cache key to delete *...

(QB_NEW_EN_OTHER)


[grammar] ~455-~455: Use correct spacing
Context: ...* - key (required): The cache key to delete cURL Example: ```bash curl -X DELETE ...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~457-~457: There might be a mistake here.
Context: ...uired): The cache key to delete cURL Example: bash curl -X DELETE http://localhost:8080/api/redis/user-session-abc123 Response: ```json { "status": ...

(QB_NEW_EN_OTHER)


[grammar] ~463-~463: There might be a mistake here.
Context: ...80/api/redis/user-session-abc123 **Response:** json { "status": "success", "message": "Redis cache deleted successfully" } ``` Notes: - For streaming respon...

(QB_NEW_EN_OTHER)


[grammar] ~472-~472: There might be a mistake here.
Context: ...is cache deleted successfully" } ``` Notes: - For streaming responses, this deletes a...

(QB_NEW_EN_OTHER)


[grammar] ~474-~474: There might be a mistake here.
Context: ...es all chunks associated with the cache key - Use the bifrost_cache_key from cached ...

(QB_NEW_EN_OTHER)


[grammar] ~475-~475: There might be a mistake here.
Context: ...ched response metadata to get the exact key - Returns success even if the key doesn't ...

(QB_NEW_EN_OTHER)


[grammar] ~476-~476: There might be a mistake here.
Context: ...Returns success even if the key doesn't exist --- ## 🔧 Request Parameters ### **Common Para...

(QB_NEW_EN_OTHER)

🪛 GitHub Actions: Snyk checks
transports/bifrost-http/main.go

[error] 87-87: Go build failed: pattern all:ui: no matching files found (bifrost-http/main.go:87:12).

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
🔇 Additional comments (5)
transports/bifrost-http/lib/config.go (1)

18-18: LGTM!

The EnableCaching field addition follows established patterns and is properly documented with an appropriate JSON tag.

plugins/redis/README.md (1)

169-169: Excellent documentation for per-request TTL feature.

The documentation clearly explains the new CacheTTLKey functionality with practical examples and proper fallback behavior description.

docs/usage/http-transport/endpoints.md (1)

414-477: Well-documented Redis cache management section

The documentation comprehensively covers the Redis caching functionality with clear examples and explanations. The cache control headers, DELETE endpoint, and cache behavior are all properly documented.

transports/bifrost-http/lib/store.go (2)

318-320: LGTM: EnableCaching correctly loaded from DB

Field mapping looks correct and consistent with ClientConfig.


699-701: LGTM: EnableCaching correctly persisted to DB

Field is persisted alongside existing client config fields.

Comment thread transports/bifrost-http/handlers/redis.go Outdated
Comment thread transports/bifrost-http/handlers/redis.go Outdated
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 08-08-feat_redis_plugin_added_to_transport branch from ad8f0c1 to abbee56 Compare August 11, 2025 11:58
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-11-_plugins_feat_redis_caching_plugin_added branch 2 times, most recently from 0707466 to c15f154 Compare August 11, 2025 12:12
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 08-08-feat_redis_plugin_added_to_transport branch 3 times, most recently from 1d02dd4 to 653f2b3 Compare August 11, 2025 12:42
akshaydeo
akshaydeo previously approved these changes Aug 11, 2025
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 08-08-feat_redis_plugin_added_to_transport branch from 94ce2e3 to 3bf0921 Compare August 11, 2025 14:26
@Pratham-Mishra04 Pratham-Mishra04 changed the base branch from graphite-base/237 to 06-11-_plugins_feat_redis_caching_plugin_added August 11, 2025 14:26
coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 11, 2025
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-11-_plugins_feat_redis_caching_plugin_added branch from d0561aa to 5216e81 Compare August 11, 2025 14:31
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 08-08-feat_redis_plugin_added_to_transport branch from 3bf0921 to c938aa0 Compare August 11, 2025 14:31
@akshaydeo akshaydeo changed the base branch from 06-11-_plugins_feat_redis_caching_plugin_added to graphite-base/237 August 11, 2025 15:02
@akshaydeo akshaydeo changed the base branch from graphite-base/237 to main August 11, 2025 15:02
@akshaydeo akshaydeo dismissed stale reviews from coderabbitai[bot] and themself August 11, 2025 15:02

The base branch was changed.

akshaydeo
akshaydeo previously approved these changes Aug 11, 2025
coderabbitai[bot]
coderabbitai Bot previously requested changes Aug 11, 2025
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🔭 Outside diff range comments (1)
transports/bifrost-http/main.go (1)

87-87: Fix embed directive syntax error.

The pipeline failure indicates the embed pattern is incorrect. The all: prefix is not valid for embed directives.

Apply this fix:

-//go:embed all:ui
+//go:embed ui/*

Or if you need to include all subdirectories:

-//go:embed all:ui
+//go:embed ui
♻️ Duplicate comments (5)
transports/bifrost-http/lib/ctx.go (1)

117-141: Well-implemented cache header processing.

The implementation correctly handles both cache headers:

  • x-bf-cache-key: Direct string storage in context
  • x-bf-cache-ttl: Flexible parsing supporting both Go duration format ("30s") and plain seconds ("300")

The dual parsing approach for TTL provides good user experience by accepting multiple input formats. The silent fallback on parsing errors is appropriate as it allows the system to use default TTL values.

transports/bifrost-http/main.go (2)

444-445: Hard-coded cache key prevents per-request control.

The CacheKey and CacheTTLKey are hard-coded, which prevents dynamic per-request cache key control as described in the PR objectives (using x-bf-cache-key header).

Consider making these configurable or deriving them from request headers to enable per-request cache control.


433-436: Service crashes if Redis config is missing.

When EnableCaching is true but no Redis config exists (first-time enable), the service will crash with log.Fatal.

Consider handling the missing config case gracefully:

 cacheDBConfig, err := store.GetCacheConfig()
 if err != nil {
-    logger.Fatal("failed to get cache config", err)
+    if errors.Is(err, gorm.ErrRecordNotFound) {
+        logger.Warn("Redis caching enabled but no config found - using defaults")
+        // Use default config or skip Redis initialization
+        return
+    }
+    logger.Fatal("failed to get cache config", err)
 }
ui/components/config/cache-config-form.tsx (1)

69-69: Use proper tuple destructuring syntax.

-      const [_, error] = await apiService.updateCacheConfig(newConfig)
+      const [, error] = await apiService.updateCacheConfig(newConfig)
transports/bifrost-http/lib/store.go (1)

65-66: Consider documenting the EnableCaching field behavior.

Based on the retrieved learnings, changes to EnableCaching require a service restart to take effect. This important behavior should be documented.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 94ce2e3 and c938aa0.

⛔ Files ignored due to path filters (1)
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (15)
  • docs/usage/http-transport/endpoints.md (1 hunks)
  • plugins/redis/README.md (1 hunks)
  • transports/bifrost-http/handlers/cache.go (1 hunks)
  • transports/bifrost-http/handlers/config.go (1 hunks)
  • transports/bifrost-http/lib/config.go (1 hunks)
  • transports/bifrost-http/lib/ctx.go (2 hunks)
  • transports/bifrost-http/lib/models.go (2 hunks)
  • transports/bifrost-http/lib/store.go (6 hunks)
  • transports/bifrost-http/main.go (3 hunks)
  • transports/go.mod (3 hunks)
  • ui/app/config/page.tsx (3 hunks)
  • ui/components/config/cache-config-form.tsx (1 hunks)
  • ui/lib/api.ts (2 hunks)
  • ui/lib/types/config.ts (1 hunks)
  • ui/lib/utils/validation.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (23)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: transports/bifrost-http/handlers/redis.go:64-67
Timestamp: 2025-08-11T12:15:34.778Z
Learning: In the Bifrost project, when configuring Redis settings, Pratham-Mishra04 prefers to let Redis itself handle validation of configuration parameters (like database numbers) rather than adding application-level validation, as Redis will return appropriate errors for invalid configurations and this approach avoids hard-coding assumptions about Redis setup limits.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#70
File: plugins/redis/main.go:480-504
Timestamp: 2025-08-11T14:31:10.852Z
Learning: In the Bifrost Redis plugin (plugins/redis/main.go), Pratham-Mishra04 prefers to keep JSON marshaling inside the async goroutine in PostHook to avoid blocking the request path, accepting potential data race risks in favor of maintaining non-blocking performance.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: ui/app/config/page.tsx:318-346
Timestamp: 2025-08-11T12:16:16.163Z
Learning: In the Bifrost UI (ui/app/config/page.tsx), the Redis configuration form is intentionally shown only when both `configInDB.enable_caching` and `config.enable_caching` are true, requiring users to enable caching and restart the service before they can configure Redis settings. This two-step process is an intended design decision.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: transports/bifrost-http/handlers/config.go:88-89
Timestamp: 2025-08-11T12:23:01.157Z
Learning: In the Bifrost transport configuration (transports/bifrost-http/handlers/config.go), changes to the `EnableCaching` field require a service restart to take effect. This is an intended design decision - the Redis caching plugin is only initialized once at startup in main.go and there's no hot-reload mechanism for enabling/disabling caching at runtime.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
📚 Learning: 2025-08-11T12:23:01.157Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: transports/bifrost-http/handlers/config.go:88-89
Timestamp: 2025-08-11T12:23:01.157Z
Learning: In the Bifrost transport configuration (transports/bifrost-http/handlers/config.go), changes to the `EnableCaching` field require a service restart to take effect. This is an intended design decision - the Redis caching plugin is only initialized once at startup in main.go and there's no hot-reload mechanism for enabling/disabling caching at runtime.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/handlers/cache.go
  • transports/bifrost-http/lib/config.go
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
  • docs/usage/http-transport/endpoints.md
📚 Learning: 2025-08-11T12:16:16.163Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: ui/app/config/page.tsx:318-346
Timestamp: 2025-08-11T12:16:16.163Z
Learning: In the Bifrost UI (ui/app/config/page.tsx), the Redis configuration form is intentionally shown only when both `configInDB.enable_caching` and `config.enable_caching` are true, requiring users to enable caching and restart the service before they can configure Redis settings. This two-step process is an intended design decision.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • ui/lib/types/config.ts
  • ui/components/config/cache-config-form.tsx
  • transports/bifrost-http/main.go
  • ui/app/config/page.tsx
  • ui/lib/api.ts
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-08-11T12:34:17.569Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: transports/bifrost-http/lib/ctx.go:16-16
Timestamp: 2025-08-11T12:34:17.569Z
Learning: In the Bifrost codebase, using string literals directly with `redis.ContextKey()` (e.g., `redis.ContextKey("request-cache-key")`) is an intended design pattern rather than exporting constants from the redis plugin. This approach is preferred by Pratham-Mishra04 for cache-related context keys.

Applied to files:

  • transports/bifrost-http/handlers/config.go
  • ui/components/config/cache-config-form.tsx
  • plugins/redis/README.md
  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/ctx.go
  • ui/lib/api.ts
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-08-11T14:24:35.506Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: ui/components/config/cache-config-form.tsx:160-168
Timestamp: 2025-08-11T14:24:35.506Z
Learning: In the Bifrost project, password fields in configuration forms (like the Redis cache configuration) are redacted by the backend when fetched. The backend returns a redacted value (e.g., "REDACTED") and preserves the existing password when this redacted value is sent back during updates. Frontend components don't need additional password masking logic as security is handled at the backend level.

Applied to files:

  • transports/bifrost-http/handlers/cache.go
  • ui/lib/types/config.ts
  • ui/components/config/cache-config-form.tsx
  • ui/lib/api.ts
📚 Learning: 2025-08-11T12:15:34.778Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: transports/bifrost-http/handlers/redis.go:64-67
Timestamp: 2025-08-11T12:15:34.778Z
Learning: In the Bifrost project, when configuring Redis settings, Pratham-Mishra04 prefers to let Redis itself handle validation of configuration parameters (like database numbers) rather than adding application-level validation, as Redis will return appropriate errors for invalid configurations and this approach avoids hard-coding assumptions about Redis setup limits.

Applied to files:

  • ui/components/config/cache-config-form.tsx
  • ui/lib/utils/validation.ts
  • ui/app/config/page.tsx
  • ui/lib/api.ts
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-07-08T17:16:50.811Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#148
File: transports/bifrost-http/lib/store.go:1081-1098
Timestamp: 2025-07-08T17:16:50.811Z
Learning: Pratham-Mishra04 prefers practical redaction approaches over theoretical security improvements when the threat model is low-risk, such as admin-only interfaces in the Bifrost project. Fixed-length redaction is acceptable when only trusted administrators will see the redacted values.

Applied to files:

  • ui/components/config/cache-config-form.tsx
📚 Learning: 2025-07-16T07:13:29.496Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: transports/bifrost-http/integrations/genai/types.go:0-0
Timestamp: 2025-07-16T07:13:29.496Z
Learning: Pratham-Mishra04 prefers to avoid redundant error handling across architectural layers in the Bifrost streaming implementation. When error handling (such as timeouts, context cancellation, and JSON marshaling failures) is already handled at the provider level, they prefer not to duplicate this logic at the transport integration layer to keep the code simple and avoid unnecessary complexity.

Applied to files:

  • ui/lib/utils/validation.ts
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-07-08T17:31:44.662Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#145
File: transports/bifrost-http/main.go:124-128
Timestamp: 2025-07-08T17:31:44.662Z
Learning: Pratham-Mishra04 prefers to keep the CORS middleware simple in the Bifrost HTTP transport (transports/bifrost-http/main.go) rather than adding port validation for localhost origins, considering the current implementation sufficient for the intended use case.

Applied to files:

  • ui/lib/utils/validation.ts
📚 Learning: 2025-08-03T20:28:00.857Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#202
File: transports/bifrost-http/handlers/governance.go:1038-1049
Timestamp: 2025-08-03T20:28:00.857Z
Learning: In the Bifrost governance handler (transports/bifrost-http/handlers/governance.go), Pratham-Mishra04 is comfortable with exposing full error details including err.Error() in production responses for governance API endpoints, prioritizing simplicity over potential information disclosure concerns.

Applied to files:

  • ui/lib/utils/validation.ts
📚 Learning: 2025-06-20T16:21:18.912Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.

Applied to files:

  • transports/go.mod
📚 Learning: 2025-08-11T14:31:10.852Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#70
File: plugins/redis/main.go:480-504
Timestamp: 2025-08-11T14:31:10.852Z
Learning: In the Bifrost Redis plugin (plugins/redis/main.go), Pratham-Mishra04 prefers to keep JSON marshaling inside the async goroutine in PostHook to avoid blocking the request path, accepting potential data race risks in favor of maintaining non-blocking performance.

Applied to files:

  • transports/go.mod
  • transports/bifrost-http/lib/ctx.go
  • ui/lib/api.ts
  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-08-11T12:34:58.738Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: transports/bifrost-http/lib/store.go:2085-2102
Timestamp: 2025-08-11T12:34:58.738Z
Learning: In the Bifrost HTTP transport's ConfigStore (transports/bifrost-http/lib/store.go), the Redis configuration table is designed to have at most one row. The UpdateRedisConfig method ensures this by deleting all existing configs before inserting a new one within a transaction, making duplicate Redis configs impossible. Using First() without ordering is safe for fetching the Redis config.

Applied to files:

  • transports/bifrost-http/main.go
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-07-08T18:20:24.086Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.

Applied to files:

  • transports/bifrost-http/main.go
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and always returns a pointer to a valid context. It starts with context.Background() and only adds values to it, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-09T11:27:00.925Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-06-17T14:04:21.104Z
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
📚 Learning: 2025-08-08T14:12:14.754Z
Learnt from: TejasGhatte
PR: maximhq/bifrost#238
File: transports/bifrost-http/plugins/logging/main.go:0-0
Timestamp: 2025-08-08T14:12:14.754Z
Learning: In transports/bifrost-http/plugins/logging/main.go, stream accumulator cleanup is intended to use a 5-minute TTL (consistent with processing logs). Comments should state “older than 5 minutes,” and using central constants (processingLogTTL, streamAccumulatorTTL) helps avoid future mismatches.

Applied to files:

  • transports/bifrost-http/lib/ctx.go
  • docs/usage/http-transport/endpoints.md
📚 Learning: 2025-07-08T18:30:08.258Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#141
File: core/bifrost.go:198-272
Timestamp: 2025-07-08T18:30:08.258Z
Learning: Pratham-Mishra04 follows a pattern of implementing core functionality first and deferring non-critical improvements (like race condition fixes, optimizations) to later PRs. This is a reasonable development approach that prioritizes getting the main feature working before addressing edge cases.

Applied to files:

  • ui/app/config/page.tsx
  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-08-11T12:26:13.812Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#237
File: transports/bifrost-http/lib/models.go:133-147
Timestamp: 2025-08-11T12:26:13.812Z
Learning: In the Bifrost SQLite database configuration (transports/bifrost-http/lib/models.go), GORM default tags are intentionally omitted for boolean fields like CacheByModel and CacheByProvider in DBRedisConfig because values are always explicitly passed when creating or updating records, rather than relying on database defaults.

Applied to files:

  • transports/bifrost-http/lib/store.go
  • transports/bifrost-http/lib/models.go
📚 Learning: 2025-08-11T11:22:28.400Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#254
File: transports/bifrost-http/integrations/utils.go:0-0
Timestamp: 2025-08-11T11:22:28.400Z
Learning: In the Bifrost HTTP transport, reading ClientConfig fields directly without mutex protection is acceptable by design. Stale values for a few milliseconds during runtime configuration updates are tolerated to avoid mutex overhead on read-heavy paths, as long as nil panics are prevented (which they are since ConfigStore is guaranteed non-nil).

Applied to files:

  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-06-04T05:37:59.699Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.

Applied to files:

  • transports/bifrost-http/lib/store.go
📚 Learning: 2025-07-09T04:58:08.229Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#150
File: transports/bifrost-http/lib/store.go:370-466
Timestamp: 2025-07-09T04:58:08.229Z
Learning: Pratham-Mishra04 prefers not to add logging or error handling for unreachable code paths in the Bifrost project. When provider types or similar entities are predefined in the system, defensive programming like logging in default cases is considered unnecessary overhead.

Applied to files:

  • transports/bifrost-http/lib/store.go
🪛 GitHub Actions: Snyk checks
transports/bifrost-http/main.go

[error] 87-87: pattern all:ui: no matching files found

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
🔇 Additional comments (15)
transports/bifrost-http/lib/config.go (1)

18-18: LGTM! Clean addition of caching configuration.

The EnableCaching field follows the established pattern for configuration options and aligns with the broader Redis caching implementation.

transports/go.mod (1)

12-12: Redis plugin dependencies properly configured.

The Redis plugin dependency and replace directives are correctly set up for local development. The indirect dependencies for Redis client operations are appropriately included.

Also applies to: 20-23, 46-46, 68-68

docs/usage/http-transport/endpoints.md (1)

414-497: Comprehensive Redis caching documentation added.

The documentation thoroughly covers the new caching capabilities including:

  • Cache control headers with clear examples
  • TTL format flexibility (Go duration vs plain seconds)
  • Cache behavior explanation
  • DELETE endpoint for cache management

This provides users with clear guidance on utilizing the Redis caching features.

plugins/redis/README.md (1)

169-169: Well-documented per-request TTL feature.

The CacheTTLKey configuration option is clearly documented with practical examples showing how to override TTL on a per-request basis. This aligns well with the HTTP transport integration.

ui/lib/types/config.ts (2)

129-129: LGTM! Caching configuration properly typed.

The enable_caching field addition to CoreConfig is clean and consistent with other boolean configuration options.


133-146: CacheConfig interface well-structured.

The interface properly captures all Redis configuration fields with appropriate optional typing. The structure aligns with the backend DBCacheConfig model.

ui/lib/api.ts (1)

234-250: Cache configuration API methods properly implemented.

Both getCacheConfig and updateCacheConfig methods follow the established ServiceResponse pattern and error handling approach. The response types align with the backend implementation.

ui/lib/utils/validation.ts (1)

313-468: Comprehensive Redis address validation implementation.

The isValidRedisAddress function provides robust validation for multiple Redis address formats:

  • Standard IPv4/hostname with port
  • Bracketed IPv6 addresses
  • Redis URL schemes (redis:// and rediss://)

The implementation correctly handles edge cases and includes proper port validation that prevents partial numeric matches. This will provide good user feedback for Redis configuration in the UI.

transports/bifrost-http/lib/ctx.go (1)

11-11: Appropriate imports added for cache header processing.

The strconv, time, and redis imports are necessary for the new cache header functionality.

Also applies to: 13-13, 17-17

transports/bifrost-http/handlers/config.go (1)

88-88: LGTM!

The implementation correctly persists the EnableCaching flag from the request. As per the design, changes to this flag require a service restart to take effect since the Redis plugin is only initialized at startup.

ui/app/config/page.tsx (1)

318-346: Implementation follows the intended design pattern.

The caching configuration UI correctly implements the two-step process:

  1. Users enable caching (triggering restart warning)
  2. After restart, Redis configuration becomes available

The conditional rendering and restart warning are properly implemented.

transports/bifrost-http/lib/models.go (1)

113-113: Database models are well-structured.

The EnableCaching field addition to DBClientConfig and the new DBCacheConfig struct properly model the Redis configuration requirements. The intentional omission of GORM defaults for boolean fields aligns with the design where values are always explicitly passed.

Also applies to: 134-146, 155-155

ui/components/config/cache-config-form.tsx (1)

160-168: Password field implementation is correct.

The password field correctly relies on backend redaction for security. The backend returns redacted values and preserves existing passwords when the redacted value is sent back, as intended by design.

transports/bifrost-http/handlers/cache.go (1)

16-31: LGTM!

The struct definition and constructor are well-structured with appropriate dependencies.

transports/bifrost-http/lib/store.go (1)

2125-2134: Transaction implementation looks good.

The atomic delete-and-create pattern within a transaction properly ensures data consistency.

Comment thread transports/bifrost-http/handlers/cache.go Outdated
Comment thread transports/bifrost-http/handlers/cache.go Outdated
Comment thread transports/bifrost-http/handlers/cache.go
Comment thread transports/bifrost-http/lib/store.go Outdated
@akshaydeo akshaydeo force-pushed the 08-08-feat_redis_plugin_added_to_transport branch from c938aa0 to 1a09cac Compare August 11, 2025 15:38
akshaydeo
akshaydeo previously approved these changes Aug 11, 2025
yashdamani
yashdamani previously approved these changes Aug 11, 2025
@akshaydeo akshaydeo dismissed stale reviews from yashdamani and themself via 78ee9ed August 11, 2025 15:48
@akshaydeo akshaydeo force-pushed the 08-08-feat_redis_plugin_added_to_transport branch from 1a09cac to 78ee9ed Compare August 11, 2025 15:48
@akshaydeo akshaydeo merged commit 6d0ece5 into main Aug 11, 2025
4 of 8 checks passed
@akshaydeo akshaydeo deleted the 08-08-feat_redis_plugin_added_to_transport branch August 31, 2025 17:28
akshaydeo added a commit that referenced this pull request Nov 17, 2025
…_transport

feat: add Redis caching plugin to transports with UI configuration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants